From b4306803f8bafed55880e7511de23e0816b0c310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= <93934272+Stefan-Ethernal@users.noreply.github.com> Date: Thu, 27 Jul 2023 09:50:16 +0200 Subject: [PATCH] Correctly initialize use allow list and use block list parameters on the access list predicates (#1750) * Split use allow list and use block list parameters * Update child chain mintable bridge test to rely on block list --- consensus/polybft/contracts_initializer.go | 30 +++++++++++----------- consensus/polybft/polybft.go | 23 ++++++++++++----- e2e-polybft/e2e/bridge_test.go | 17 ++++++------ 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/consensus/polybft/contracts_initializer.go b/consensus/polybft/contracts_initializer.go index 16239b8f6c..fe4a36ddfa 100644 --- a/consensus/polybft/contracts_initializer.go +++ b/consensus/polybft/contracts_initializer.go @@ -85,7 +85,7 @@ func getInitERC20PredicateInput(config *BridgeConfig, childChainMintable bool) ( // getInitERC20PredicateACLInput builds initialization input parameters for child chain ERC20PredicateAccessList SC func getInitERC20PredicateACLInput(config *BridgeConfig, owner types.Address, - childChainMintable bool) ([]byte, error) { + useAllowList, useBlockList, childChainMintable bool) ([]byte, error) { var params contractsapi.StateTransactionInput if childChainMintable { params = &contractsapi.InitializeRootMintableERC20PredicateACLFn{ @@ -93,8 +93,8 @@ func getInitERC20PredicateACLInput(config *BridgeConfig, owner types.Address, NewStateReceiver: contracts.StateReceiverContract, NewChildERC20Predicate: config.ChildMintableERC20PredicateAddr, NewChildTokenTemplate: config.ChildERC20Addr, - NewUseAllowList: owner != contracts.SystemCaller, - NewUseBlockList: owner != contracts.SystemCaller, + NewUseAllowList: useAllowList, + NewUseBlockList: useBlockList, NewOwner: owner, } } else { @@ -104,8 +104,8 @@ func getInitERC20PredicateACLInput(config *BridgeConfig, owner types.Address, NewRootERC20Predicate: config.RootERC20PredicateAddr, NewChildTokenTemplate: contracts.ChildERC20Contract, NewNativeTokenRootAddress: config.RootNativeERC20Addr, - NewUseAllowList: owner != contracts.SystemCaller, - NewUseBlockList: owner != contracts.SystemCaller, + NewUseAllowList: useAllowList, + NewUseBlockList: useBlockList, NewOwner: owner, } } @@ -138,7 +138,7 @@ func getInitERC721PredicateInput(config *BridgeConfig, childOriginatedTokens boo // getInitERC721PredicateACLInput builds initialization input parameters // for child chain ERC721PredicateAccessList SC func getInitERC721PredicateACLInput(config *BridgeConfig, owner types.Address, - childChainMintable bool) ([]byte, error) { + useAllowList, useBlockList, childChainMintable bool) ([]byte, error) { var params contractsapi.StateTransactionInput if childChainMintable { params = &contractsapi.InitializeRootMintableERC721PredicateACLFn{ @@ -146,8 +146,8 @@ func getInitERC721PredicateACLInput(config *BridgeConfig, owner types.Address, NewStateReceiver: contracts.StateReceiverContract, NewChildERC721Predicate: config.ChildMintableERC721PredicateAddr, NewChildTokenTemplate: config.ChildERC721Addr, - NewUseAllowList: owner != contracts.SystemCaller, - NewUseBlockList: owner != contracts.SystemCaller, + NewUseAllowList: useAllowList, + NewUseBlockList: useBlockList, NewOwner: owner, } } else { @@ -156,8 +156,8 @@ func getInitERC721PredicateACLInput(config *BridgeConfig, owner types.Address, NewStateReceiver: contracts.StateReceiverContract, NewRootERC721Predicate: config.RootERC721PredicateAddr, NewChildTokenTemplate: contracts.ChildERC721Contract, - NewUseAllowList: owner != contracts.SystemCaller, - NewUseBlockList: owner != contracts.SystemCaller, + NewUseAllowList: useAllowList, + NewUseBlockList: useBlockList, NewOwner: owner, } } @@ -190,7 +190,7 @@ func getInitERC1155PredicateInput(config *BridgeConfig, childChainMintable bool) // getInitERC1155PredicateACLInput builds initialization input parameters // for child chain ERC1155PredicateAccessList SC func getInitERC1155PredicateACLInput(config *BridgeConfig, owner types.Address, - childChainMintable bool) ([]byte, error) { + useAllowList, useBlockList, childChainMintable bool) ([]byte, error) { var params contractsapi.StateTransactionInput if childChainMintable { params = &contractsapi.InitializeRootMintableERC1155PredicateACLFn{ @@ -198,8 +198,8 @@ func getInitERC1155PredicateACLInput(config *BridgeConfig, owner types.Address, NewStateReceiver: contracts.StateReceiverContract, NewChildERC1155Predicate: config.ChildMintableERC1155PredicateAddr, NewChildTokenTemplate: config.ChildERC1155Addr, - NewUseAllowList: owner != contracts.SystemCaller, - NewUseBlockList: owner != contracts.SystemCaller, + NewUseAllowList: useAllowList, + NewUseBlockList: useBlockList, NewOwner: owner, } } else { @@ -208,8 +208,8 @@ func getInitERC1155PredicateACLInput(config *BridgeConfig, owner types.Address, NewStateReceiver: contracts.StateReceiverContract, NewRootERC1155Predicate: config.RootERC1155PredicateAddr, NewChildTokenTemplate: contracts.ChildERC1155Contract, - NewUseAllowList: owner != contracts.SystemCaller, - NewUseBlockList: owner != contracts.SystemCaller, + NewUseAllowList: useAllowList, + NewUseBlockList: useBlockList, NewOwner: owner, } } diff --git a/consensus/polybft/polybft.go b/consensus/polybft/polybft.go index bca3fa49e5..8f832d0e75 100644 --- a/consensus/polybft/polybft.go +++ b/consensus/polybft/polybft.go @@ -167,7 +167,7 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st // 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 + bridgeAllowListAdmin := types.ZeroAddress if config.Params.BridgeAllowList != nil && len(config.Params.BridgeAllowList.AdminAddresses) > 0 { bridgeAllowListAdmin = config.Params.BridgeAllowList.AdminAddresses[0] } @@ -181,6 +181,9 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st 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 + useBridgeAllowList := bridgeAllowListAdmin != types.ZeroAddress + useBridgeBlockList := bridgeBlockListAdmin != types.ZeroAddress + if bridgeAllowListAdmin != types.ZeroAddress { owner = bridgeAllowListAdmin } else if bridgeBlockListAdmin != types.ZeroAddress { @@ -188,7 +191,8 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st } // initialize ChildERC20PredicateAccessList SC - input, err := getInitERC20PredicateACLInput(polyBFTConfig.Bridge, owner, false) + input, err := getInitERC20PredicateACLInput(polyBFTConfig.Bridge, owner, + useBridgeAllowList, useBridgeBlockList, false) if err != nil { return err } @@ -199,7 +203,8 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st } // initialize ChildERC721PredicateAccessList SC - input, err = getInitERC721PredicateACLInput(polyBFTConfig.Bridge, owner, false) + input, err = getInitERC721PredicateACLInput(polyBFTConfig.Bridge, owner, + useBridgeAllowList, useBridgeBlockList, false) if err != nil { return err } @@ -210,7 +215,8 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st } // initialize ChildERC1155PredicateAccessList SC - input, err = getInitERC1155PredicateACLInput(polyBFTConfig.Bridge, owner, false) + input, err = getInitERC1155PredicateACLInput(polyBFTConfig.Bridge, owner, + useBridgeAllowList, useBridgeBlockList, false) if err != nil { return err } @@ -221,7 +227,8 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st } // initialize RootMintableERC20PredicateAccessList SC - input, err = getInitERC20PredicateACLInput(polyBFTConfig.Bridge, owner, true) + input, err = getInitERC20PredicateACLInput(polyBFTConfig.Bridge, owner, + useBridgeAllowList, useBridgeBlockList, true) if err != nil { return err } @@ -232,7 +239,8 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st } // initialize RootMintableERC721PredicateAccessList SC - input, err = getInitERC721PredicateACLInput(polyBFTConfig.Bridge, owner, true) + input, err = getInitERC721PredicateACLInput(polyBFTConfig.Bridge, owner, + useBridgeAllowList, useBridgeBlockList, true) if err != nil { return err } @@ -243,7 +251,8 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st } // initialize RootMintableERC1155PredicateAccessList SC - input, err = getInitERC1155PredicateACLInput(polyBFTConfig.Bridge, owner, true) + input, err = getInitERC1155PredicateACLInput(polyBFTConfig.Bridge, owner, + useBridgeAllowList, useBridgeBlockList, true) if err != nil { return err } diff --git a/e2e-polybft/e2e/bridge_test.go b/e2e-polybft/e2e/bridge_test.go index 96cf94ece9..97354b2639 100644 --- a/e2e-polybft/e2e/bridge_test.go +++ b/e2e-polybft/e2e/bridge_test.go @@ -675,7 +675,6 @@ func TestE2E_Bridge_ChildChainMintableTokensTransfer(t *testing.T) { framework.WithNumBlockConfirmations(0), framework.WithEpochSize(epochSize), framework.WithNativeTokenConfig(fmt.Sprintf(nativeTokenMintableTestCfg, adminAddr)), - framework.WithBridgeAllowListAdmin(adminAddr), framework.WithBridgeBlockListAdmin(adminAddr), framework.WithPremine(append(depositors, adminAddr)...)) //nolint:makezero defer cluster.Stop() @@ -703,8 +702,11 @@ func TestE2E_Bridge_ChildChainMintableTokensTransfer(t *testing.T) { // rootToken represents deposit token (basically native mintable token from the Supernets) rootToken := contracts.NativeERC20TokenContract + // block list first depositor + setAccessListRole(t, cluster, contracts.BlockListBridgeAddr, depositors[0], addresslist.EnabledRole, admin) + // try sending a single native token deposit transaction - // it should fail, because depositors are not allow listed for bridge transactions + // it should fail, because first depositor is added to bridge transactions block list err = cluster.Bridge.Deposit( common.ERC20, rootToken, @@ -718,11 +720,11 @@ func TestE2E_Bridge_ChildChainMintableTokensTransfer(t *testing.T) { true) require.Error(t, err) + // remove first depositor from bridge transactions block list + setAccessListRole(t, cluster, contracts.BlockListBridgeAddr, depositors[0], addresslist.NoRole, admin) + // allow list each depositor and make sure deposit is successfully executed for i, key := range depositorKeys { - // add all depositors to bridge allow list - setAccessListRole(t, cluster, contracts.AllowListBridgeAddr, depositors[i], addresslist.EnabledRole, admin) - // make sure deposit is successfully executed err = cluster.Bridge.Deposit( common.ERC20, @@ -859,9 +861,6 @@ func TestE2E_Bridge_ChildChainMintableTokensTransfer(t *testing.T) { require.Error(t, err) for i, depositorKey := range depositorKeys { - // add all depositors to the bridge allow list - setAccessListRole(t, cluster, contracts.AllowListBridgeAddr, depositors[i], addresslist.EnabledRole, admin) - // remove all depositors from the bridge block list setAccessListRole(t, cluster, contracts.BlockListBridgeAddr, depositors[i], addresslist.NoRole, admin) @@ -1284,7 +1283,7 @@ func TestE2E_Bridge_Transfers_AccessLists(t *testing.T) { // add account to bridge block list setAccessListRole(t, cluster, contracts.BlockListBridgeAddr, senderAccount.Address(), addresslist.EnabledRole, admin) - // it should fail now because in block list + // it should fail now because sender accont is in the block list err = cluster.Bridge.Withdraw( common.ERC20, hex.EncodeToString(rawKey),