From 02d721db8332da3345eaac98fa78d1256050133d Mon Sep 17 00:00:00 2001 From: krehermann <16602512+krehermann@users.noreply.github.com> Date: Mon, 16 Dec 2024 10:17:24 -0700 Subject: [PATCH 1/6] revoke deployer key from timelock admin --- .../transfer_to_mcms_with_timelock.go | 32 +++++++++++++++ .../transfer_to_mcms_with_timelock_test.go | 41 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock.go b/deployment/common/changeset/transfer_to_mcms_with_timelock.go index 1980dfaa378..d5ebe0be8c9 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock.go @@ -211,3 +211,35 @@ func TransferToDeployer(e deployment.Environment, cfg TransferToDeployerConfig) e.Logger.Infof("deployer key accepted ownership tx %s", tx.Hash().Hex()) return deployment.ChangesetOutput{}, nil } + +var _ deployment.ChangeSet[RevokeTimelockDeployerConfig] = RevokeTimelockDeployer + +type RevokeTimelockDeployerConfig struct { + ContractAddress common.Address + ChainSel uint64 +} + +// RevokeTimelockDeployer revokes the deployer key from administering the contract. +func RevokeTimelockDeployer(e deployment.Environment, cfg RevokeTimelockDeployerConfig) (deployment.ChangesetOutput, error) { + contracts, err := MaybeLoadMCMSWithTimelockState(e, []uint64{cfg.ChainSel}) + if err != nil { + return deployment.ChangesetOutput{}, err + } + tl := contracts[cfg.ChainSel].Timelock + if tl == nil { + return deployment.ChangesetOutput{}, fmt.Errorf("timelock not found on chain %d", cfg.ChainSel) + } + admin, err := tl.ADMINROLE(&bind.CallOpts{}) + if err != nil { + return deployment.ChangesetOutput{}, fmt.Errorf("failed to get admin role: %w", err) + } + tx, err := tl.RevokeRole(e.Chains[cfg.ChainSel].DeployerKey, admin, e.Chains[cfg.ChainSel].DeployerKey.From) + if err != nil { + return deployment.ChangesetOutput{}, fmt.Errorf("failed to revoke deployer key: %w", err) + } + if _, err := deployment.ConfirmIfNoError(e.Chains[cfg.ChainSel], tx, err); err != nil { + return deployment.ChangesetOutput{}, err + } + e.Logger.Infof("revoked deployer key from owning contract %s", cfg.ContractAddress) + return deployment.ChangesetOutput{}, nil +} diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go index daf4309398f..87576d0b270 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go @@ -1,8 +1,10 @@ package changeset import ( + "encoding/hex" "testing" + "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" @@ -78,3 +80,42 @@ func TestTransferToMCMSWithTimelock(t *testing.T) { require.NoError(t, err) require.Equal(t, e.Chains[chain1].DeployerKey.From, o) } + +func TestRevokeTimelockDeployer(t *testing.T) { + lggr := logger.TestLogger(t) + e := memory.NewMemoryEnvironment(t, lggr, 0, memory.MemoryEnvironmentConfig{ + Chains: 1, + Nodes: 1, + }) + chain1 := e.AllChainSelectors()[0] + e, err := ApplyChangesets(t, e, nil, []ChangesetApplication{ + { + Changeset: WrapChangeSet(DeployMCMSWithTimelock), + Config: map[uint64]types.MCMSWithTimelockConfig{ + chain1: proposalutils.SingleGroupTimelockConfig(t), + }, + }, + }) + require.NoError(t, err) + addrs, err := e.ExistingAddresses.AddressesForChain(chain1) + require.NoError(t, err) + + state, err := MaybeLoadMCMSWithTimelockChainState(e.Chains[chain1], addrs) + require.NoError(t, err) + + tl := state.Timelock + require.NotNil(t, tl) + + // WHAT AM I DOING HERE? HOW DO I TEST? + adminRole, err := tl.ADMINROLE(nil) + require.NoError(t, err) + + r, err := tl.GetRoleAdmin(&bind.CallOpts{}, adminRole) + require.NoError(t, err) + h := hex.EncodeToString(r[:]) + t.Logf("admin role: %s", h) + a := common.BytesToAddress(r[:]) + + require.Equal(t, a, e.Chains[chain1].DeployerKey.From) + +} From 6e43f5d2c42b2199505f96b1351790aee2536cfe Mon Sep 17 00:00:00 2001 From: Akhil Chainani Date: Mon, 16 Dec 2024 12:47:40 -0500 Subject: [PATCH 2/6] use RenounceRole instead of Revoke --- .../transfer_to_mcms_with_timelock.go | 15 ++++++------ .../transfer_to_mcms_with_timelock_test.go | 24 +++++++++++++------ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock.go b/deployment/common/changeset/transfer_to_mcms_with_timelock.go index d5ebe0be8c9..afba1afa48b 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock.go @@ -212,15 +212,14 @@ func TransferToDeployer(e deployment.Environment, cfg TransferToDeployerConfig) return deployment.ChangesetOutput{}, nil } -var _ deployment.ChangeSet[RevokeTimelockDeployerConfig] = RevokeTimelockDeployer +var _ deployment.ChangeSet[RenounceTimelockDeployerConfig] = RenounceTimelockDeployer -type RevokeTimelockDeployerConfig struct { - ContractAddress common.Address - ChainSel uint64 +type RenounceTimelockDeployerConfig struct { + ChainSel uint64 } -// RevokeTimelockDeployer revokes the deployer key from administering the contract. -func RevokeTimelockDeployer(e deployment.Environment, cfg RevokeTimelockDeployerConfig) (deployment.ChangesetOutput, error) { +// RenounceTimelockDeployer revokes the deployer key from administering the contract. +func RenounceTimelockDeployer(e deployment.Environment, cfg RenounceTimelockDeployerConfig) (deployment.ChangesetOutput, error) { contracts, err := MaybeLoadMCMSWithTimelockState(e, []uint64{cfg.ChainSel}) if err != nil { return deployment.ChangesetOutput{}, err @@ -233,13 +232,13 @@ func RevokeTimelockDeployer(e deployment.Environment, cfg RevokeTimelockDeployer if err != nil { return deployment.ChangesetOutput{}, fmt.Errorf("failed to get admin role: %w", err) } - tx, err := tl.RevokeRole(e.Chains[cfg.ChainSel].DeployerKey, admin, e.Chains[cfg.ChainSel].DeployerKey.From) + tx, err := tl.RenounceRole(e.Chains[cfg.ChainSel].DeployerKey, admin, e.Chains[cfg.ChainSel].DeployerKey.From) if err != nil { return deployment.ChangesetOutput{}, fmt.Errorf("failed to revoke deployer key: %w", err) } if _, err := deployment.ConfirmIfNoError(e.Chains[cfg.ChainSel], tx, err); err != nil { return deployment.ChangesetOutput{}, err } - e.Logger.Infof("revoked deployer key from owning contract %s", cfg.ContractAddress) + e.Logger.Infof("revoked deployer key from owning contract %s", tl.Address().Hex()) return deployment.ChangesetOutput{}, nil } diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go index 87576d0b270..74b5685f455 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go @@ -1,7 +1,6 @@ package changeset import ( - "encoding/hex" "testing" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -81,7 +80,7 @@ func TestTransferToMCMSWithTimelock(t *testing.T) { require.Equal(t, e.Chains[chain1].DeployerKey.From, o) } -func TestRevokeTimelockDeployer(t *testing.T) { +func TestRenounceTimelockDeployer(t *testing.T) { lggr := logger.TestLogger(t) e := memory.NewMemoryEnvironment(t, lggr, 0, memory.MemoryEnvironmentConfig{ Chains: 1, @@ -110,12 +109,23 @@ func TestRevokeTimelockDeployer(t *testing.T) { adminRole, err := tl.ADMINROLE(nil) require.NoError(t, err) - r, err := tl.GetRoleAdmin(&bind.CallOpts{}, adminRole) + r, err := tl.GetRoleMemberCount(&bind.CallOpts{}, adminRole) require.NoError(t, err) - h := hex.EncodeToString(r[:]) - t.Logf("admin role: %s", h) - a := common.BytesToAddress(r[:]) + require.Equal(t, 2, r.Int64()) - require.Equal(t, a, e.Chains[chain1].DeployerKey.From) + // Revoke Deployer + e, err = ApplyChangesets(t, e, nil, []ChangesetApplication{ + { + Changeset: WrapChangeSet(RenounceTimelockDeployer), + Config: RenounceTimelockDeployerConfig{ + ChainSel: chain1, + }, + }, + }) + require.NoError(t, err) + // Check that the deployer is no longer an admin + r, err = tl.GetRoleMemberCount(&bind.CallOpts{}, adminRole) + require.NoError(t, err) + require.Equal(t, 1, r.Int64()) } From 2d9ad8755ba5d6707d30ed924dabfc46f880a3d1 Mon Sep 17 00:00:00 2001 From: Akhil Chainani Date: Mon, 16 Dec 2024 13:19:55 -0500 Subject: [PATCH 3/6] add addtional checks/tests --- .../changeset/transfer_to_mcms_with_timelock_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go index 74b5685f455..7dba028c758 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go @@ -1,6 +1,7 @@ package changeset import ( + "math/big" "testing" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -128,4 +129,11 @@ func TestRenounceTimelockDeployer(t *testing.T) { r, err = tl.GetRoleMemberCount(&bind.CallOpts{}, adminRole) require.NoError(t, err) require.Equal(t, 1, r.Int64()) + + // Retrive the admin address + admin, err := tl.GetRoleMember(&bind.CallOpts{}, adminRole, big.NewInt(0)) + require.NoError(t, err) + + // Check that the admin is the timelock + require.Equal(t, tl.Address(), admin) } From 5d41fdfc6071d0eecfd2ebdc304db9ffa02e5e0f Mon Sep 17 00:00:00 2001 From: Akhil Chainani Date: Mon, 16 Dec 2024 13:26:38 -0500 Subject: [PATCH 4/6] fix int type casting in test --- .../common/changeset/transfer_to_mcms_with_timelock_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go index 7dba028c758..d50852784c2 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go @@ -112,7 +112,7 @@ func TestRenounceTimelockDeployer(t *testing.T) { r, err := tl.GetRoleMemberCount(&bind.CallOpts{}, adminRole) require.NoError(t, err) - require.Equal(t, 2, r.Int64()) + require.Equal(t, int64(2), r.Int64()) // Revoke Deployer e, err = ApplyChangesets(t, e, nil, []ChangesetApplication{ @@ -128,7 +128,7 @@ func TestRenounceTimelockDeployer(t *testing.T) { // Check that the deployer is no longer an admin r, err = tl.GetRoleMemberCount(&bind.CallOpts{}, adminRole) require.NoError(t, err) - require.Equal(t, 1, r.Int64()) + require.Equal(t, int64(1), r.Int64()) // Retrive the admin address admin, err := tl.GetRoleMember(&bind.CallOpts{}, adminRole, big.NewInt(0)) From ed2bab7367413c97aa9dd05961299a55382faf9b Mon Sep 17 00:00:00 2001 From: Akhil Chainani Date: Mon, 16 Dec 2024 13:43:19 -0500 Subject: [PATCH 5/6] remove comment --- .../common/changeset/transfer_to_mcms_with_timelock_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go index d50852784c2..6434aa8a380 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go @@ -106,7 +106,6 @@ func TestRenounceTimelockDeployer(t *testing.T) { tl := state.Timelock require.NotNil(t, tl) - // WHAT AM I DOING HERE? HOW DO I TEST? adminRole, err := tl.ADMINROLE(nil) require.NoError(t, err) From f36a5585f9fa4038b4259614767b64093d622808 Mon Sep 17 00:00:00 2001 From: Akhil Chainani Date: Mon, 16 Dec 2024 13:51:47 -0500 Subject: [PATCH 6/6] fix typo --- .../common/changeset/transfer_to_mcms_with_timelock_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go index 6434aa8a380..49c46aab5f0 100644 --- a/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go +++ b/deployment/common/changeset/transfer_to_mcms_with_timelock_test.go @@ -129,7 +129,7 @@ func TestRenounceTimelockDeployer(t *testing.T) { require.NoError(t, err) require.Equal(t, int64(1), r.Int64()) - // Retrive the admin address + // Retrieve the admin address admin, err := tl.GetRoleMember(&bind.CallOpts{}, adminRole, big.NewInt(0)) require.NoError(t, err)