From a92f5008169271b484f9edeb2ae645c61bf075bf Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Wed, 23 Nov 2022 12:19:58 -0500 Subject: [PATCH 1/5] working draft --- BEP174.md | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 BEP174.md diff --git a/BEP174.md b/BEP174.md new file mode 100644 index 00000000..fa5c3c48 --- /dev/null +++ b/BEP174.md @@ -0,0 +1,137 @@ +# BEP-174: Cross Chain Relayer Management + +## 1. Summary + +This BEP introduces a new governance proposal type to manage the set of whitelisted Relayers. A Relayer whitelist was introduced recently to the BSC genesis contracts ([PR-198](https://github.com/bnb-chain/bsc-genesis-contract/pull/198)). This BEP will improve the management of the relayer whitelist by introducing Relayer Managers, where a Relayer Manager can manage the registration of a single Relayer. Managers will be elected and ejected via governance. + +## 2. Abstract + +This BEP introduces a new governance proposal type to manage Relayer Manager public keys that will be stored in the [`RelayerHub`](https://github.com/bnb-chain/bsc-genesis-contract/blob/71dcd4c409a68a6e084645a9f1f80adecd5a4269/contracts/RelayerHub.sol) genesis contract on BSC. + +Relayer Managers will be able to manage their individual elayer public key. Only Relayer public keys registered via Managers will be able to invoke the `RelayerHub` smart contract for cross chain transfers. + + +## 3. Status + +This BEP is a draft. + +## 4. Motivation + +After the [BSC Bridge exploitation](https://github.com/verichains/public-audit-reports/discussions/17) it is important we introduce some mechanism in place that enhances the security of the system. A governance based set Manager public keys who manage a single Relayer adds an extra layer of security. + + +## 5. Specification + + +### 5.1 Current Relayer Whitelist + +Currently whitelisted relayers are [hardcoded](https://github.com/bnb-chain/bsc-genesis-contract/blob/a8476b2aefba0a66db86311ab44d43d7e2df24fe/contracts/System.sol#L88) in the `System` genesis smart contract. This list needs to be updated manually by deploying a new version of the smart contract. + +### 5.2 The New Whitelisting Mechanism +After the implementation of this BEP: + +- A Relayer Manager role will be introduced with corresponding public keys. +- The set of Managers will be updated via governance using the existing [param change proposal](https://github.com/bnb-chain/bnb-chain.github.io/blob/master/docs/learn/bsc-gov-workflow.md#submit-cross-chain-param-change-proposal) mechanism. +- Managers will have to register the 100BNB once their public key has been added to the set of Managers via governance +- Once registered, Managers will be able to add an associated Relayer public key +- There will be a 1:1 relationship between Manager public key to active Relayer public key. +- Only associated Relayers can send cross chain messages. +- Managers can modify public key of their Relayer at any time. This will facilitate rotation of Relayer instances. + +### 5.3 `RelayerHub` Smart Contract Changes +#### Relayer Manager functions +``` +function removeManagerByGov(address) external onlyGov +``` +Removes Manager public key from current set of Managers. 100BNB will be returned to Manager address if the Manager has desposited. Emits an event on success. This is only called via governance. +``` +function removeManager() external onlyManager +``` +Removes Manager public key from current set of Managers. 100BNB will be returned to Manager address if the Manager has desposited. Emits an event on success. +``` +fuction addManagerByGov(address) external onlyGov +``` +Adds a Manager public key to the current set of Managers. This is only called via governance. Emits an event on success. + +``` +function registerManager() external payable onlyManager +``` +Registers an Manager and accepts 100BNB fee. Emits an event on success. + +``` +function addRelayer(address) external onlyManager +``` +Adds a Relayer for the calling Manager. Overwrites Relayer if there is an existing Relayer already added. Emits an event on success. + +``` +function registerManagerAddRelayer(address) external payable onlyManager +``` +Calls both `registerManager` and `addRelayer`. + +``` +function removeRelayer() external onlyManager +``` +Removes a Relayer associated to the calling Manager. Emits an event on successful removal. + +#### Relayer functions +``` +function verifyRelayer(address) external bool +``` +This will be called by the Relayer codebase, instead of registering. Relayer can stop if this returns false + + +#### `updateParam` Changes + +``` + function updateParam(string calldata key, bytes calldata value) external override onlyInit onlyGov{ + if (Memory.compareStrings(key,"requiredDeposit")) { + ... + } else if (Memory.compareStrings(key,"dues")) { + ... + } else if (Memory.compareStrings(key,"addManager")) { + // TODO check and parse value + // addManagerAddress(...) + } else if (Memory.compareStrings(key,"removeManager")) { + // TODO check and parse value + // removeManagerAddress(...) + } else { + require(false, "unknown param"); + } + emit paramChange(key, value); + } +``` +The `updateParam` function will be modified accept two new keys +- `addManager` to add a Manager to the stored Manager set. +- `removeManager` to remove a Manager from the stored Manager set. + +#### Functions to be Deprecated + +These Relayer functions will be deprecated: +``` +function register() external payable ... +function unregister() external ... +``` + +### 5.4 Relayer Changes + +Update Relayer codebase to no longer register and deposit. Update abi? and run code generation to call new `verifyRelayer` method on `RelayerHub` smart contract and proceed to relay cross chain messages on success. + +### 5.5 Existing relayer key migration + +Manager keys will need to be supplied for the existing two relayers. Manger set will be seeded with these two Manager public keys. Relayer public keys will be registered to these Managers. + +Existing Relayers will need to pull down relayer code once the `RelayerHub` SC has been upgraded. + +### 5.6 CLI commands +``` +## mainet +./bnbcli params submit-bscParam-change-proposal --key "addManager" --value "ADDRESS" --target 0x0000000000000000000000000000000000001006 --deposit 200000000000:BNB --voting-period 100 --side-chain-id bsc --title "add new super trusty relayer" --from alice --trust-node --chain-id Binance-Chain-Tigris + +## testnet +./tbnbcli params submit-bscParam-change-proposal --key "addManager" --value "ADDRESS" --target 0x0000000000000000000000000000000000001006 --deposit 200000000000:BNB --voting-period 100 --side-chain-id chapel --title "add new super trusty relayer" --from alice --trust-node --chain-id Binance-Chain-Ganges +``` +The commands used to create a change proposal to governance. + +## 7. License + +The content is licensed under [CC0](https://creativecommons.org/publicdomain/zero/1.0/). \ No newline at end of file From 877c4dd6469b5823ebfd69d77e787db282187550 Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Thu, 24 Nov 2022 08:50:47 -0500 Subject: [PATCH 2/5] update relayer fn --- BEP174.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BEP174.md b/BEP174.md index fa5c3c48..3734701f 100644 --- a/BEP174.md +++ b/BEP174.md @@ -75,7 +75,7 @@ Removes a Relayer associated to the calling Manager. Emits an event on successf #### Relayer functions ``` -function verifyRelayer(address) external bool +function isRelayer(address) external bool ``` This will be called by the Relayer codebase, instead of registering. Relayer can stop if this returns false From ec524cf47928b7bfd324d3b0f2d49b7840d96870 Mon Sep 17 00:00:00 2001 From: Satyajit Das Date: Mon, 23 Jan 2023 11:14:46 +0000 Subject: [PATCH 3/5] Update BEP-174 with newer function changes --- BEP174.md | 64 +++++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/BEP174.md b/BEP174.md index 3734701f..7e9634df 100644 --- a/BEP174.md +++ b/BEP174.md @@ -2,18 +2,21 @@ ## 1. Summary -This BEP introduces a new governance proposal type to manage the set of whitelisted Relayers. A Relayer whitelist was introduced recently to the BSC genesis contracts ([PR-198](https://github.com/bnb-chain/bsc-genesis-contract/pull/198)). This BEP will improve the management of the relayer whitelist by introducing Relayer Managers, where a Relayer Manager can manage the registration of a single Relayer. Managers will be elected and ejected via governance. +This BEP introduces a new governance proposal type to manage the set of whitelisted Relayers. +A Relayer whitelist was introduced recently to the BSC genesis contracts ([PR-198](https://github.com/bnb-chain/bsc-genesis-contract/pull/198)). +This BEP will improve the management of the relayer whitelist by introducing Relayer Managers, where a Relayer Manager can manage the registration of a single Relayer. +Managers will be elected and ejected via governance. ## 2. Abstract This BEP introduces a new governance proposal type to manage Relayer Manager public keys that will be stored in the [`RelayerHub`](https://github.com/bnb-chain/bsc-genesis-contract/blob/71dcd4c409a68a6e084645a9f1f80adecd5a4269/contracts/RelayerHub.sol) genesis contract on BSC. -Relayer Managers will be able to manage their individual elayer public key. Only Relayer public keys registered via Managers will be able to invoke the `RelayerHub` smart contract for cross chain transfers. +Relayer Managers will be able to manage their individual relayer public key. Only Relayer public keys registered via Managers will be able to invoke the `RelayerHub` smart contract for cross chain transfers. ## 3. Status -This BEP is a draft. +This BEP is under progress. See https://github.com/bnb-chain/bsc-genesis-contract/pull/205. ## 4. Motivation @@ -32,50 +35,48 @@ After the implementation of this BEP: - A Relayer Manager role will be introduced with corresponding public keys. - The set of Managers will be updated via governance using the existing [param change proposal](https://github.com/bnb-chain/bnb-chain.github.io/blob/master/docs/learn/bsc-gov-workflow.md#submit-cross-chain-param-change-proposal) mechanism. -- Managers will have to register the 100BNB once their public key has been added to the set of Managers via governance -- Once registered, Managers will be able to add an associated Relayer public key +- Managers will NOT have to deposit any BNB (as opposed to 100BNB requirement which was before). +- Once selected by governance, Managers will be able to add an associated Relayer public key. - There will be a 1:1 relationship between Manager public key to active Relayer public key. - Only associated Relayers can send cross chain messages. -- Managers can modify public key of their Relayer at any time. This will facilitate rotation of Relayer instances. +- Managers can modify public key of their Relayer at any time. This will facilitate rotation of Relayer instances. +- The old `unregister` function will still be active to allow the existing whitelisted relayers to safely exit. ### 5.3 `RelayerHub` Smart Contract Changes #### Relayer Manager functions + ``` -function removeManagerByGov(address) external onlyGov -``` -Removes Manager public key from current set of Managers. 100BNB will be returned to Manager address if the Manager has desposited. Emits an event on success. This is only called via governance. -``` -function removeManager() external onlyManager -``` -Removes Manager public key from current set of Managers. 100BNB will be returned to Manager address if the Manager has desposited. Emits an event on success. -``` -fuction addManagerByGov(address) external onlyGov +function updateParam(string calldata key, bytes calldata value) external override onlyInit onlyGov ``` -Adds a Manager public key to the current set of Managers. This is only called via governance. Emits an event on success. +The pre-existing function will be updated to support `addManager` and `removeManager` keys to facilitate adding and +removing of managers via governance. ``` -function registerManager() external payable onlyManager +function removeManagerByHimself() external ``` -Registers an Manager and accepts 100BNB fee. Emits an event on success. +Allows a manager to remove himself if required. ``` -function addRelayer(address) external onlyManager +fuction addManagerByGov(address) external onlyGov ``` -Adds a Relayer for the calling Manager. Overwrites Relayer if there is an existing Relayer already added. Emits an event on success. +Adds a Manager public key to the current set of Managers. This is only called via governance. Emits an event on success. ``` -function registerManagerAddRelayer(address) external payable onlyManager +function updateRelayer(address relayerToBeAdded) public onlyManager ``` -Calls both `registerManager` and `addRelayer`. +This can be used by the manager to add or remove its relayer. Overwrites Relayer if there is an existing Relayer already +added. Emits an event on success. The `relayerToBeAdded` parameter can be set to `0` address to enable removal of a +relayer (e.g. if the private key gets compromised). ``` -function removeRelayer() external onlyManager +function whitelistInit() external ``` -Removes a Relayer associated to the calling Manager. Emits an event on successful removal. +This is to ensure smooth transition so that the presently existing hardcoded whitelisted relayers continue to be +relayers after the hardfork. This function needs to be **manually** called once after the hardfork. #### Relayer functions ``` -function isRelayer(address) external bool +function isRelayer(address relayerAddress) external override view returns (bool) ``` This will be called by the Relayer codebase, instead of registering. Relayer can stop if this returns false @@ -84,11 +85,7 @@ This will be called by the Relayer codebase, instead of registering. Relayer ca ``` function updateParam(string calldata key, bytes calldata value) external override onlyInit onlyGov{ - if (Memory.compareStrings(key,"requiredDeposit")) { - ... - } else if (Memory.compareStrings(key,"dues")) { - ... - } else if (Memory.compareStrings(key,"addManager")) { + if (Memory.compareStrings(key,"addManager")) { // TODO check and parse value // addManagerAddress(...) } else if (Memory.compareStrings(key,"removeManager")) { @@ -104,14 +101,17 @@ The `updateParam` function will be modified accept two new keys - `addManager` to add a Manager to the stored Manager set. - `removeManager` to remove a Manager from the stored Manager set. +Also `requiredDeposit` and `dues` will be removed. So no need to account for them in the keys check. + #### Functions to be Deprecated These Relayer functions will be deprecated: ``` function register() external payable ... -function unregister() external ... ``` +`unregister()` function isn't deprecated to allow existing hardcoded whitelisted relayers to have safe exit. + ### 5.4 Relayer Changes Update Relayer codebase to no longer register and deposit. Update abi? and run code generation to call new `verifyRelayer` method on `RelayerHub` smart contract and proceed to relay cross chain messages on success. @@ -134,4 +134,4 @@ The commands used to create a change proposal to governance. ## 7. License -The content is licensed under [CC0](https://creativecommons.org/publicdomain/zero/1.0/). \ No newline at end of file +The content is licensed under [CC0](https://creativecommons.org/publicdomain/zero/1.0/). From 98e4e209f83fda4155b3e4fe768cb774746ef8e4 Mon Sep 17 00:00:00 2001 From: Satyajit Das Date: Tue, 24 Jan 2023 13:07:24 +0000 Subject: [PATCH 4/5] Address pr suggestion --- BEP174.md | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/BEP174.md b/BEP174.md index 7e9634df..7d387ec4 100644 --- a/BEP174.md +++ b/BEP174.md @@ -35,7 +35,7 @@ After the implementation of this BEP: - A Relayer Manager role will be introduced with corresponding public keys. - The set of Managers will be updated via governance using the existing [param change proposal](https://github.com/bnb-chain/bnb-chain.github.io/blob/master/docs/learn/bsc-gov-workflow.md#submit-cross-chain-param-change-proposal) mechanism. -- Managers will NOT have to deposit any BNB (as opposed to 100BNB requirement which was before). +- Managers will NOT have to deposit any BNB (as opposed to the previous 100BNB deposit requirement). - Once selected by governance, Managers will be able to add an associated Relayer public key. - There will be a 1:1 relationship between Manager public key to active Relayer public key. - Only associated Relayers can send cross chain messages. @@ -48,8 +48,7 @@ After the implementation of this BEP: ``` function updateParam(string calldata key, bytes calldata value) external override onlyInit onlyGov ``` -The pre-existing function will be updated to support `addManager` and `removeManager` keys to facilitate adding and -removing of managers via governance. +The pre-existing function will be updated to support `addManager` and `removeManager` keys to facilitate adding and removing of managers via governance. ``` function removeManagerByHimself() external @@ -64,15 +63,12 @@ Adds a Manager public key to the current set of Managers. This is only called vi ``` function updateRelayer(address relayerToBeAdded) public onlyManager ``` -This can be used by the manager to add or remove its relayer. Overwrites Relayer if there is an existing Relayer already -added. Emits an event on success. The `relayerToBeAdded` parameter can be set to `0` address to enable removal of a -relayer (e.g. if the private key gets compromised). +This can be used by the manager to add or remove its relayer. Overwrites Relayer if there is an existing Relayer already added. Emits an event on success. The `relayerToBeAdded` parameter can be set to `0` address to enable removal of a relayer (e.g. if the private key gets compromised). ``` function whitelistInit() external ``` -This is to ensure smooth transition so that the presently existing hardcoded whitelisted relayers continue to be -relayers after the hardfork. This function needs to be **manually** called once after the hardfork. +This is to ensure smooth transition so that the presently existing hardcoded whitelisted relayers continue to be relayers after the hardfork. This function needs to be **manually** called once after the hardfork. #### Relayer functions ``` From bf88d9cf514b0082f788182ff5c3ed8c475c1296 Mon Sep 17 00:00:00 2001 From: Matus Kysel Date: Wed, 1 Mar 2023 09:34:14 +0100 Subject: [PATCH 5/5] fix broken url and some params --- BEP174.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/BEP174.md b/BEP174.md index 7d387ec4..5ec072a3 100644 --- a/BEP174.md +++ b/BEP174.md @@ -1,3 +1,11 @@ +
+  BEP: 174
+  Title: Cross Chain Relayer Management
+  Status: Draft
+  Type: Standards
+  Created: 2022-11-23
+
+ # BEP-174: Cross Chain Relayer Management ## 1. Summary @@ -34,7 +42,7 @@ Currently whitelisted relayers are [hardcoded](https://github.com/bnb-chain/bsc- After the implementation of this BEP: - A Relayer Manager role will be introduced with corresponding public keys. -- The set of Managers will be updated via governance using the existing [param change proposal](https://github.com/bnb-chain/bnb-chain.github.io/blob/master/docs/learn/bsc-gov-workflow.md#submit-cross-chain-param-change-proposal) mechanism. +- The set of Managers will be updated via governance using the existing [param change proposal](https://github.com/bnb-chain/bnb-chain.github.io/blob/45f59e2/docs/learn/bsc-gov.md#submit-cross-chain-param-change-proposal) mechanism. - Managers will NOT have to deposit any BNB (as opposed to the previous 100BNB deposit requirement). - Once selected by governance, Managers will be able to add an associated Relayer public key. - There will be a 1:1 relationship between Manager public key to active Relayer public key. @@ -121,10 +129,10 @@ Existing Relayers will need to pull down relayer code once the `RelayerHub` SC h ### 5.6 CLI commands ``` ## mainet -./bnbcli params submit-bscParam-change-proposal --key "addManager" --value "ADDRESS" --target 0x0000000000000000000000000000000000001006 --deposit 200000000000:BNB --voting-period 100 --side-chain-id bsc --title "add new super trusty relayer" --from alice --trust-node --chain-id Binance-Chain-Tigris +./bnbcli params submit-cscParam-change-proposal --key "addManager" --value "ADDRESS" --target 0x0000000000000000000000000000000000001006 --deposit 200000000000:BNB --voting-period 100 --side-chain-id bsc --title "add new super trusty relayer" --from alice --trust-node --chain-id Binance-Chain-Tigris ## testnet -./tbnbcli params submit-bscParam-change-proposal --key "addManager" --value "ADDRESS" --target 0x0000000000000000000000000000000000001006 --deposit 200000000000:BNB --voting-period 100 --side-chain-id chapel --title "add new super trusty relayer" --from alice --trust-node --chain-id Binance-Chain-Ganges +./tbnbcli params submit-cscParam-change-proposal --key "addManager" --value "ADDRESS" --target 0x0000000000000000000000000000000000001006 --deposit 200000000000:BNB --voting-period 100 --side-chain-id chapel --title "add new super trusty relayer" --from alice --trust-node --chain-id Binance-Chain-Ganges ``` The commands used to create a change proposal to governance.