Skip to content

Commit

Permalink
Correctly initialize use allow list and use block list parameters on …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
Stefan-Ethernal authored Jul 27, 2023
1 parent bbbe3ab commit b430680
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 31 deletions.
30 changes: 15 additions & 15 deletions consensus/polybft/contracts_initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,16 @@ 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{
NewL2StateSender: contracts.L2StateSenderContract,
NewStateReceiver: contracts.StateReceiverContract,
NewChildERC20Predicate: config.ChildMintableERC20PredicateAddr,
NewChildTokenTemplate: config.ChildERC20Addr,
NewUseAllowList: owner != contracts.SystemCaller,
NewUseBlockList: owner != contracts.SystemCaller,
NewUseAllowList: useAllowList,
NewUseBlockList: useBlockList,
NewOwner: owner,
}
} else {
Expand All @@ -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,
}
}
Expand Down Expand Up @@ -138,16 +138,16 @@ 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{
NewL2StateSender: contracts.L2StateSenderContract,
NewStateReceiver: contracts.StateReceiverContract,
NewChildERC721Predicate: config.ChildMintableERC721PredicateAddr,
NewChildTokenTemplate: config.ChildERC721Addr,
NewUseAllowList: owner != contracts.SystemCaller,
NewUseBlockList: owner != contracts.SystemCaller,
NewUseAllowList: useAllowList,
NewUseBlockList: useBlockList,
NewOwner: owner,
}
} else {
Expand All @@ -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,
}
}
Expand Down Expand Up @@ -190,16 +190,16 @@ 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{
NewL2StateSender: contracts.L2StateSenderContract,
NewStateReceiver: contracts.StateReceiverContract,
NewChildERC1155Predicate: config.ChildMintableERC1155PredicateAddr,
NewChildTokenTemplate: config.ChildERC1155Addr,
NewUseAllowList: owner != contracts.SystemCaller,
NewUseBlockList: owner != contracts.SystemCaller,
NewUseAllowList: useAllowList,
NewUseBlockList: useBlockList,
NewOwner: owner,
}
} else {
Expand All @@ -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,
}
}
Expand Down
23 changes: 16 additions & 7 deletions consensus/polybft/polybft.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
Expand All @@ -181,14 +181,18 @@ 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 {
owner = bridgeBlockListAdmin
}

// initialize ChildERC20PredicateAccessList SC
input, err := getInitERC20PredicateACLInput(polyBFTConfig.Bridge, owner, false)
input, err := getInitERC20PredicateACLInput(polyBFTConfig.Bridge, owner,
useBridgeAllowList, useBridgeBlockList, false)
if err != nil {
return err
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
17 changes: 8 additions & 9 deletions e2e-polybft/e2e/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit b430680

Please sign in to comment.