From 7747e121ac519ad10999f5af1bc5c46ef383548e Mon Sep 17 00:00:00 2001 From: Victor Castell Date: Tue, 9 May 2023 17:53:36 +0200 Subject: [PATCH] Remove bridge allow/block list admin vars (#1482) * Specific error for bridge test * Remove BridgeAllowListAdmin BridgeBlockListAdmin param It's redundant as we already have all necessary data in chain config. * Update core-contracts branch --- command/genesis/polybft_params.go | 22 ++------- consensus/polybft/contracts_initializer.go | 56 +++++++--------------- consensus/polybft/polybft.go | 29 +++++++++-- consensus/polybft/polybft_config.go | 6 --- core-contracts | 2 +- e2e-polybft/e2e/bridge_test.go | 2 +- 6 files changed, 46 insertions(+), 71 deletions(-) diff --git a/command/genesis/polybft_params.go b/command/genesis/polybft_params.go index 26b9a79a72..57a326740b 100644 --- a/command/genesis/polybft_params.go +++ b/command/genesis/polybft_params.go @@ -122,18 +122,6 @@ func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) er } } - // check if there are Bridge Allow List Admins and Bridge Block List Admins - // and if there are, get the first address as the Admin - var bridgeAllowListAdmin types.Address - if len(p.bridgeAllowListAdmin) > 0 { - bridgeAllowListAdmin = types.StringToAddress(p.bridgeAllowListAdmin[0]) - } - - var bridgeBlockListAdmin types.Address - if len(p.bridgeBlockListAdmin) > 0 { - bridgeBlockListAdmin = types.StringToAddress(p.bridgeBlockListAdmin[0]) - } - polyBftConfig := &polybft.PolyBFTConfig{ InitialValidatorSet: initialValidators, BlockTime: common.Duration{Duration: p.blockTime}, @@ -141,12 +129,10 @@ func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) er SprintSize: p.sprintSize, EpochReward: p.epochReward, // use 1st account as governance address - Governance: initialValidators[0].Address, - InitialTrieRoot: types.StringToHash(p.initialStateRoot), - NativeTokenConfig: p.nativeTokenConfig, - BridgeAllowListAdmin: bridgeAllowListAdmin, - BridgeBlockListAdmin: bridgeBlockListAdmin, - MaxValidatorSetSize: p.maxNumValidators, + Governance: initialValidators[0].Address, + InitialTrieRoot: types.StringToHash(p.initialStateRoot), + NativeTokenConfig: p.nativeTokenConfig, + MaxValidatorSetSize: p.maxNumValidators, RewardConfig: &polybft.RewardsConfig{ TokenAddress: rewardTokenAddr, WalletAddress: walletPremineInfo.address, diff --git a/consensus/polybft/contracts_initializer.go b/consensus/polybft/contracts_initializer.go index 9776466a00..8c1688b329 100644 --- a/consensus/polybft/contracts_initializer.go +++ b/consensus/polybft/contracts_initializer.go @@ -77,7 +77,7 @@ func getInitChildERC20PredicateInput(config *BridgeConfig) ([]byte, error) { } // getInitChildERC20PredicateAccessListInput builds input parameters for ChildERC20PredicateAccessList SC initialization -func getInitChildERC20PredicateAccessListInput(config PolyBFTConfig) ([]byte, error) { +func getInitChildERC20PredicateAccessListInput(config *BridgeConfig, owner types.Address) ([]byte, error) { //nolint:godox // to be fixed with EVM-541 // TODO: @Stefan-Ethernal Temporary workaround just to be able to run cluster in non-bridge mode, until SC is fixed @@ -86,17 +86,9 @@ func getInitChildERC20PredicateAccessListInput(config PolyBFTConfig) ([]byte, er //nolint:godox // TODO: This can be removed as we'll always have a bridge config - if config.Bridge != nil { - rootERC20PredicateAddr = config.Bridge.RootERC20PredicateAddr - rootERC20Addr = config.Bridge.RootNativeERC20Addr - } - - // The owner of the contract will be the allow list admin or the block list admin, if any of them is set. - owner := contracts.SystemCaller - if config.BridgeAllowListAdmin != types.ZeroAddress { - owner = config.BridgeAllowListAdmin - } else if config.BridgeBlockListAdmin != types.ZeroAddress { - owner = config.BridgeBlockListAdmin + if config != nil { + rootERC20PredicateAddr = config.RootERC20PredicateAddr + rootERC20Addr = config.RootNativeERC20Addr } params := &contractsapi.InitializeChildERC20PredicateAccessListFn{ @@ -105,8 +97,8 @@ func getInitChildERC20PredicateAccessListInput(config PolyBFTConfig) ([]byte, er NewRootERC20Predicate: rootERC20PredicateAddr, NewChildTokenTemplate: contracts.ChildERC20Contract, NewNativeTokenRootAddress: rootERC20Addr, - UseAllowList: config.BridgeAllowListAdmin != types.ZeroAddress, - UseBlockList: config.BridgeBlockListAdmin != types.ZeroAddress, + UseAllowList: owner != contracts.SystemCaller, + UseBlockList: owner != contracts.SystemCaller, NewOwner: owner, } @@ -133,19 +125,11 @@ func getInitChildERC721PredicateInput(config *BridgeConfig) ([]byte, error) { // getInitChildERC721PredicateAccessListInput builds input parameters // for ChildERC721PredicateAccessList SC initialization -func getInitChildERC721PredicateAccessListInput(config PolyBFTConfig) ([]byte, error) { +func getInitChildERC721PredicateAccessListInput(config *BridgeConfig, owner types.Address) ([]byte, error) { rootERC721PredicateAccessListAddr := types.StringToAddress(disabledBridgeRootPredicateAddr) - if config.Bridge != nil { - rootERC721PredicateAccessListAddr = config.Bridge.RootERC721PredicateAddr - } - - // The owner of the contract will be the allow list admin or the block list admin, if any of them is set. - owner := contracts.SystemCaller - if config.BridgeAllowListAdmin != types.ZeroAddress { - owner = config.BridgeAllowListAdmin - } else if config.BridgeBlockListAdmin != types.ZeroAddress { - owner = config.BridgeBlockListAdmin + if config != nil { + rootERC721PredicateAccessListAddr = config.RootERC721PredicateAddr } params := &contractsapi.InitializeChildERC721PredicateAccessListFn{ @@ -153,8 +137,8 @@ func getInitChildERC721PredicateAccessListInput(config PolyBFTConfig) ([]byte, e NewStateReceiver: contracts.StateReceiverContract, NewRootERC721Predicate: rootERC721PredicateAccessListAddr, NewChildTokenTemplate: contracts.ChildERC721Contract, - UseAllowList: config.BridgeAllowListAdmin != types.ZeroAddress, - UseBlockList: config.BridgeBlockListAdmin != types.ZeroAddress, + UseAllowList: owner != contracts.SystemCaller, + UseBlockList: owner != contracts.SystemCaller, NewOwner: owner, } @@ -181,19 +165,11 @@ func getInitChildERC1155PredicateInput(config *BridgeConfig) ([]byte, error) { // getInitChildERC1155PredicateAccessListInput builds input parameters // for ChildERC1155PredicateAccessList SC initialization -func getInitChildERC1155PredicateAccessListInput(config PolyBFTConfig) ([]byte, error) { +func getInitChildERC1155PredicateAccessListInput(config *BridgeConfig, owner types.Address) ([]byte, error) { rootERC1155PredicateAccessListAddr := types.StringToAddress(disabledBridgeRootPredicateAddr) - if config.Bridge != nil { - rootERC1155PredicateAccessListAddr = config.Bridge.RootERC1155PredicateAddr - } - - // The owner of the contract will be the allow list admin or the block list admin, if any of them is set. - owner := contracts.SystemCaller - if config.BridgeAllowListAdmin != types.ZeroAddress { - owner = config.BridgeAllowListAdmin - } else if config.BridgeBlockListAdmin != types.ZeroAddress { - owner = config.BridgeBlockListAdmin + if config != nil { + rootERC1155PredicateAccessListAddr = config.RootERC1155PredicateAddr } params := &contractsapi.InitializeChildERC1155PredicateAccessListFn{ @@ -201,8 +177,8 @@ func getInitChildERC1155PredicateAccessListInput(config PolyBFTConfig) ([]byte, NewStateReceiver: contracts.StateReceiverContract, NewRootERC1155Predicate: rootERC1155PredicateAccessListAddr, NewChildTokenTemplate: contracts.ChildERC1155Contract, - UseAllowList: config.BridgeAllowListAdmin != types.ZeroAddress, - UseBlockList: config.BridgeBlockListAdmin != types.ZeroAddress, + UseAllowList: owner != contracts.SystemCaller, + UseBlockList: owner != contracts.SystemCaller, NewOwner: owner, } diff --git a/consensus/polybft/polybft.go b/consensus/polybft/polybft.go index b79940ac4f..a2e97fa462 100644 --- a/consensus/polybft/polybft.go +++ b/consensus/polybft/polybft.go @@ -144,10 +144,29 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st return err } + // check if there are Bridge Allow List Admins and Bridge Block List Admins + // and if there are, get the first address as the Admin + var bridgeAllowListAdmin types.Address + if config.Params.BridgeAllowList != nil && len(config.Params.BridgeAllowList.AdminAddresses) > 0 { + bridgeAllowListAdmin = config.Params.BridgeAllowList.AdminAddresses[0] + } + + var bridgeBlockListAdmin types.Address + if config.Params.BridgeBlockList != nil && len(config.Params.BridgeBlockList.AdminAddresses) > 0 { + bridgeBlockListAdmin = config.Params.BridgeBlockList.AdminAddresses[0] + } + // initialize Predicate SCs - if polyBFTConfig.BridgeAllowListAdmin != types.ZeroAddress || - polyBFTConfig.BridgeBlockListAdmin != types.ZeroAddress { - input, err = getInitChildERC20PredicateAccessListInput(polyBFTConfig) + if bridgeAllowListAdmin != types.ZeroAddress || bridgeBlockListAdmin != types.ZeroAddress { + // The owner of the contract will be the allow list admin or the block list admin, if any of them is set. + owner := contracts.SystemCaller + if bridgeAllowListAdmin != types.ZeroAddress { + owner = bridgeAllowListAdmin + } else if bridgeBlockListAdmin != types.ZeroAddress { + owner = bridgeBlockListAdmin + } + + input, err = getInitChildERC20PredicateAccessListInput(polyBFTConfig.Bridge, owner) if err != nil { return err } @@ -157,7 +176,7 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st return err } - input, err = getInitChildERC721PredicateAccessListInput(polyBFTConfig) + input, err = getInitChildERC721PredicateAccessListInput(polyBFTConfig.Bridge, owner) if err != nil { return err } @@ -167,7 +186,7 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st return err } - input, err = getInitChildERC1155PredicateAccessListInput(polyBFTConfig) + input, err = getInitChildERC1155PredicateAccessListInput(polyBFTConfig.Bridge, owner) if err != nil { return err } diff --git a/consensus/polybft/polybft_config.go b/consensus/polybft/polybft_config.go index 6cdc5e789a..ceef7cd7f1 100644 --- a/consensus/polybft/polybft_config.go +++ b/consensus/polybft/polybft_config.go @@ -40,12 +40,6 @@ type PolyBFTConfig struct { // NativeTokenConfig defines name, symbol and decimal count of the native token NativeTokenConfig *TokenConfig `json:"nativeTokenConfig"` - // BridgeAllowListAdmin indicates whether bridge allow list is active - BridgeAllowListAdmin types.Address `json:"bridgeAllowListAdmin"` - - // BridgeBlockListAdmin indicates whether bridge block list is active - BridgeBlockListAdmin types.Address `json:"bridgeBlockListAdmin"` - InitialTrieRoot types.Hash `json:"initialTrieRoot"` // MaxValidatorSetSize indicates the maximum size of validator set diff --git a/core-contracts b/core-contracts index e79fa1a422..84d0721eb1 160000 --- a/core-contracts +++ b/core-contracts @@ -1 +1 @@ -Subproject commit e79fa1a42261789bef7e22a2d408ed4eaa958819 +Subproject commit 84d0721eb13a1b482e826d1a6cc53c6b45cfb291 diff --git a/e2e-polybft/e2e/bridge_test.go b/e2e-polybft/e2e/bridge_test.go index 5ba8636d53..7399d9064b 100644 --- a/e2e-polybft/e2e/bridge_test.go +++ b/e2e-polybft/e2e/bridge_test.go @@ -939,7 +939,7 @@ func TestE2E_Bridge_Transfers_AccessLists(t *testing.T) { "", validatorSrv.JSONRPCAddr(), contracts.NativeERC20TokenContract) - require.Error(t, err) + require.ErrorContains(t, err, "failed to execute withdrawal") currentBlock, err := childEthEndpoint.GetBlockByNumber(ethgo.Latest, false) require.NoError(t, err)