From 8b5919cb3126bf8c1124eab69057c234d65e0e3f Mon Sep 17 00:00:00 2001 From: Jade Park Date: Fri, 24 Jan 2025 17:59:59 +0900 Subject: [PATCH] fix: separated tests for bypasser op account --- .../contracts/tests/mcms/mcm_timelock_test.go | 42 ++------- .../mcms/timelock_bypasser_execute_test.go | 13 +-- .../tests/mcms/timelock_rbac_test.go | 90 ++++++++++++++++--- chains/solana/utils/timelock/timelock.go | 69 +++++++++++++- .../utils/timelock/timelock_operations.go | 8 +- 5 files changed, 161 insertions(+), 61 deletions(-) diff --git a/chains/solana/contracts/tests/mcms/mcm_timelock_test.go b/chains/solana/contracts/tests/mcms/mcm_timelock_test.go index 674b94126..3d158210b 100644 --- a/chains/solana/contracts/tests/mcms/mcm_timelock_test.go +++ b/chains/solana/contracts/tests/mcms/mcm_timelock_test.go @@ -351,10 +351,11 @@ func TestMcmWithTimelock(t *testing.T) { salt, sErr := timelockutil.SimpleSalt() require.NoError(t, sErr) acceptOwnershipOp := timelockutil.Operation{ - TimelockID: config.TestTimelockID, - Predecessor: config.TimelockEmptyOpID, - Salt: salt, - Delay: uint64(1), + TimelockID: config.TestTimelockID, + Predecessor: config.TimelockEmptyOpID, + Salt: salt, + Delay: uint64(1), + IsBypasserOp: true, } acceptOwnershipOp.AddInstruction(acceptOwnershipIx, []solana.PublicKey{config.McmProgram}) @@ -362,49 +363,18 @@ func TestMcmWithTimelock(t *testing.T) { id := acceptOwnershipOp.OperationID() operationPDA := acceptOwnershipOp.OperationPDA() - ixs, ierr := timelockutil.GetPreloadOperationIxs(config.TestTimelockID, acceptOwnershipOp, admin.PublicKey(), roleMsigs.AccessController.PublicKey()) + ixs, ierr := timelockutil.GetPreloadBypasserOperationIxs(config.TestTimelockID, acceptOwnershipOp, admin.PublicKey(), roleMsigs.AccessController.PublicKey()) require.NoError(t, ierr) for _, ix := range ixs { testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ix}, admin, config.DefaultCommitment) } - scheduleBatchIx, scErr := timelock.NewScheduleBatchInstruction( - config.TestTimelockID, - acceptOwnershipOp.OperationID(), - acceptOwnershipOp.Delay, - operationPDA, - timelockutil.GetConfigPDA(config.TestTimelockID), - roleMsigs.AccessController.PublicKey(), - admin.PublicKey(), - ).ValidateAndBuild() - require.NoError(t, scErr) - - tx := testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{scheduleBatchIx}, admin, config.DefaultCommitment) - parsed := common.ParseLogMessages(tx.Meta.LogMessages, - []common.EventMapping{ - common.EventMappingFor[timelockutil.CallScheduled]("CallScheduled"), - }, - ) - - for _, ixx := range acceptOwnershipOp.ToInstructionData() { - event := parsed[0].EventData[0].Data.(*timelockutil.CallScheduled) - require.Equal(t, acceptOwnershipOp.OperationID(), event.ID) - require.Equal(t, acceptOwnershipOp.Salt, event.Salt) - require.Equal(t, ixx.Data, event.Data) - } - var opAccount timelock.Operation err = common.GetAccountDataBorshInto(ctx, solanaGoClient, operationPDA, config.DefaultCommitment, &opAccount) if err != nil { require.NoError(t, err, "failed to get account info") } - require.Equal(t, - tx.BlockTime.Time().Add(time.Duration(acceptOwnershipOp.Delay)*time.Second).Unix(), - int64(opAccount.Timestamp), - "Scheduled Times don't match", - ) - require.Equal(t, id, opAccount.Id, diff --git a/chains/solana/contracts/tests/mcms/timelock_bypasser_execute_test.go b/chains/solana/contracts/tests/mcms/timelock_bypasser_execute_test.go index 3bd38a148..85bc91220 100644 --- a/chains/solana/contracts/tests/mcms/timelock_bypasser_execute_test.go +++ b/chains/solana/contracts/tests/mcms/timelock_bypasser_execute_test.go @@ -206,14 +206,15 @@ func TestTimelockBypasserExecute(t *testing.T) { require.Equal(t, strconv.Itoa(int(requiredAmount)), adminWsolBalance.Value.Amount) }) - t.Run("success: schedule and execute batch instructions", func(t *testing.T) { + t.Run("success: preload and execute batch instructions via bypasser_execute_batch", func(t *testing.T) { salt, err := timelockutil.SimpleSalt() require.NoError(t, err) op := timelockutil.Operation{ - TimelockID: config.TestTimelockID, - Predecessor: config.TimelockEmptyOpID, - Salt: salt, - Delay: uint64(1000), + TimelockID: config.TestTimelockID, + Predecessor: config.TimelockEmptyOpID, + Salt: salt, + Delay: uint64(1000), + IsBypasserOp: true, } cIx, _, ciErr := tokens.CreateAssociatedTokenAccount( @@ -246,7 +247,7 @@ func TestTimelockBypasserExecute(t *testing.T) { signer := roleMap[timelock.Bypasser_Role].RandomPick() ac := roleMap[timelock.Bypasser_Role].AccessController - ixs, err := timelockutil.GetPreloadOperationIxs(config.TestTimelockID, op, signer.PublicKey(), ac.PublicKey()) + ixs, err := timelockutil.GetPreloadBypasserOperationIxs(config.TestTimelockID, op, signer.PublicKey(), ac.PublicKey()) require.NoError(t, err) for _, ix := range ixs { testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ix}, signer, config.DefaultCommitment) diff --git a/chains/solana/contracts/tests/mcms/timelock_rbac_test.go b/chains/solana/contracts/tests/mcms/timelock_rbac_test.go index 99395c975..10c41501f 100644 --- a/chains/solana/contracts/tests/mcms/timelock_rbac_test.go +++ b/chains/solana/contracts/tests/mcms/timelock_rbac_test.go @@ -539,7 +539,7 @@ func TestTimelockRBAC(t *testing.T) { require.Equal(t, newMinDelay, newConfigAccount.MinDelay, "MinDelay is not updated") }) - t.Run("rbac: preloading operation is only accessible from proposer / bypasser(+admin)", func(t *testing.T) { + t.Run("rbac: preloading normal operation is only accessible from proposer and admin", func(t *testing.T) { cfgs := []struct { Name string Ac solana.PrivateKey @@ -552,24 +552,19 @@ func TestTimelockRBAC(t *testing.T) { Ac: roleMap[timelock.Proposer_Role].AccessController, Signer: roleMap[timelock.Proposer_Role].RandomPick(), }, - { - Name: "authorized bypasser", - Ac: roleMap[timelock.Bypasser_Role].AccessController, - Signer: roleMap[timelock.Bypasser_Role].RandomPick(), - }, { Name: "proper proposer ac, unauthorized signer", Ac: roleMap[timelock.Proposer_Role].AccessController, // valid - Signer: roleMap[timelock.Canceller_Role].RandomPick(), // unauthorized signer + Signer: roleMap[timelock.Bypasser_Role].RandomPick(), // unauthorized signer ShouldFail: true, ExpectedError: "Error Code: " + timelockutil.UnauthorizedError.String(), }, { - Name: "proper bypasser ac, unauthorized signer", - Ac: roleMap[timelock.Bypasser_Role].AccessController, // valid - Signer: roleMap[timelock.Canceller_Role].RandomPick(), // unauthorized signer + Name: "invalid access controller", + Ac: roleMap[timelock.Bypasser_Role].AccessController, // invalid access controller + Signer: roleMap[timelock.Bypasser_Role].RandomPick(), ShouldFail: true, - ExpectedError: "Error Code: " + timelockutil.UnauthorizedError.String(), + ExpectedError: "Error Code: " + timelock.InvalidAccessController_TimelockError.String(), }, { Name: "invalid access controller", @@ -616,5 +611,78 @@ func TestTimelockRBAC(t *testing.T) { }) } }) + t.Run("rbac: preloading bypasser operation is only accessible from bypasser and admin", func(t *testing.T) { + cfgs := []struct { + Name string + Ac solana.PrivateKey + Signer solana.PrivateKey + ShouldFail bool + ExpectedError string + }{ + { + Name: "authorized bypasser", + Ac: roleMap[timelock.Bypasser_Role].AccessController, + Signer: roleMap[timelock.Bypasser_Role].RandomPick(), + }, + { + Name: "proper bypasser ac, unauthorized signer", + Ac: roleMap[timelock.Bypasser_Role].AccessController, // valid + Signer: roleMap[timelock.Proposer_Role].RandomPick(), // unauthorized signer + ShouldFail: true, + ExpectedError: "Error Code: " + timelockutil.UnauthorizedError.String(), + }, + { + Name: "invalid access controller", + Ac: roleMap[timelock.Canceller_Role].AccessController, // invalid access controller + Signer: roleMap[timelock.Canceller_Role].RandomPick(), + ShouldFail: true, + ExpectedError: "Error Code: " + timelock.InvalidAccessController_TimelockError.String(), + }, + { + Name: "invalid access controller", + Ac: roleMap[timelock.Executor_Role].AccessController, // invalid access controller + Signer: roleMap[timelock.Executor_Role].RandomPick(), + ShouldFail: true, + ExpectedError: "Error Code: " + timelock.InvalidAccessController_TimelockError.String(), + }, + { + Name: "invalid access controller", + Ac: roleMap[timelock.Proposer_Role].AccessController, // invalid access controller + Signer: roleMap[timelock.Proposer_Role].RandomPick(), + ShouldFail: true, + ExpectedError: "Error Code: " + timelock.InvalidAccessController_TimelockError.String(), + }, + } + + for _, cfg := range cfgs { + t.Run(fmt.Sprintf("rbac: preloading bypasser operation test case - %s", cfg.Name), func(t *testing.T) { + t.Parallel() + signer := cfg.Signer + ac := cfg.Ac + + salt, serr := timelockutil.SimpleSalt() + require.NoError(t, serr) + op := timelockutil.Operation{ + TimelockID: config.TestTimelockID, + Predecessor: config.TimelockEmptyOpID, + Salt: salt, + Delay: uint64(1), + IsBypasserOp: true, + } + ix := system.NewTransferInstruction(1*solana.LAMPORTS_PER_SOL, admin.PublicKey(), timelockutil.GetSignerPDA(config.TestTimelockID)).Build() + op.AddInstruction(ix, []solana.PublicKey{}) + + ixs, prerr := timelockutil.GetPreloadBypasserOperationIxs(config.TestTimelockID, op, signer.PublicKey(), ac.PublicKey()) + require.NoError(t, prerr) + if cfg.ShouldFail { + testutils.SendAndFailWith(ctx, t, solanaGoClient, ixs, signer, config.DefaultCommitment, []string{cfg.ExpectedError}) + } else { + for _, ix := range ixs { + testutils.SendAndConfirm(ctx, t, solanaGoClient, []solana.Instruction{ix}, signer, config.DefaultCommitment) + } + } + }) + } + }) }) } diff --git a/chains/solana/utils/timelock/timelock.go b/chains/solana/utils/timelock/timelock.go index 1a2ca580b..d191c3880 100644 --- a/chains/solana/utils/timelock/timelock.go +++ b/chains/solana/utils/timelock/timelock.go @@ -39,6 +39,15 @@ func GetOperationPDA(timelockID [32]byte, opID [32]byte) solana.PublicKey { return pda } +func GetBypasserOperationPDA(timelockID [32]byte, opID [32]byte) solana.PublicKey { + pda, _, _ := solana.FindProgramAddress([][]byte{ + []byte("timelock_bypasser_operation"), + timelockID[:], + opID[:], + }, config.TimelockProgram) + return pda +} + // instruction builder for initializing access controllers func GetInitAccessControllersIxs(ctx context.Context, roleAcAccount solana.PublicKey, authority solana.PrivateKey, client *rpc.Client) ([]solana.Instruction, error) { ixs := []solana.Instruction{} @@ -111,7 +120,7 @@ func GetBatchAddAccessIxs(ctx context.Context, timelockID [32]byte, roleAcAccoun } // instructions builder for preloading instructions to timelock operation -func GetPreloadOperationIxs(timelockID [32]byte, op Operation, authority solana.PublicKey, accessController solana.PublicKey) ([]solana.Instruction, error) { +func GetPreloadOperationIxs(timelockID [32]byte, op Operation, authority solana.PublicKey, proposerAc solana.PublicKey) ([]solana.Instruction, error) { ixs := []solana.Instruction{} initOpIx, ioErr := timelock.NewInitializeOperationInstruction( timelockID, @@ -121,7 +130,7 @@ func GetPreloadOperationIxs(timelockID [32]byte, op Operation, authority solana. op.IxsCountU32(), op.OperationPDA(), GetConfigPDA(timelockID), - accessController, + proposerAc, authority, solana.SystemProgramID, ).ValidateAndBuild() @@ -137,7 +146,7 @@ func GetPreloadOperationIxs(timelockID [32]byte, op Operation, authority solana. []timelock.InstructionData{instruction}, // this should be a slice of instruction within 1232 bytes op.OperationPDA(), GetConfigPDA(timelockID), - accessController, + proposerAc, authority, solana.SystemProgramID, // for reallocation ).ValidateAndBuild() @@ -152,7 +161,59 @@ func GetPreloadOperationIxs(timelockID [32]byte, op Operation, authority solana. op.OperationID(), op.OperationPDA(), GetConfigPDA(timelockID), - accessController, + proposerAc, + authority, + ).ValidateAndBuild() + if foErr != nil { + return nil, fmt.Errorf("failed to build finalize operation instruction: %w", foErr) + } + ixs = append(ixs, finOpIx) + + return ixs, nil +} + +// instructions builder for preloading instructions to timelock bypasser operation +func GetPreloadBypasserOperationIxs(timelockID [32]byte, op Operation, authority solana.PublicKey, bypasserAc solana.PublicKey) ([]solana.Instruction, error) { + ixs := []solana.Instruction{} + initOpIx, ioErr := timelock.NewInitializeBypasserOperationInstruction( + timelockID, + op.OperationID(), + op.Salt, + op.IxsCountU32(), + op.OperationPDA(), + GetConfigPDA(timelockID), + bypasserAc, + authority, + solana.SystemProgramID, + ).ValidateAndBuild() + if ioErr != nil { + return nil, fmt.Errorf("failed to build initialize operation instruction: %w", ioErr) + } + ixs = append(ixs, initOpIx) + + for _, instruction := range op.ToInstructionData() { + appendIxsIx, apErr := timelock.NewAppendBypasserInstructionsInstruction( + timelockID, + op.OperationID(), + []timelock.InstructionData{instruction}, // this should be a slice of instruction within 1232 bytes + op.OperationPDA(), + GetConfigPDA(timelockID), + bypasserAc, + authority, + solana.SystemProgramID, // for reallocation + ).ValidateAndBuild() + if apErr != nil { + return nil, fmt.Errorf("failed to build append instructions instruction: %w", apErr) + } + ixs = append(ixs, appendIxsIx) + } + + finOpIx, foErr := timelock.NewFinalizeBypasserOperationInstruction( + timelockID, + op.OperationID(), + op.OperationPDA(), + GetConfigPDA(timelockID), + bypasserAc, authority, ).ValidateAndBuild() if foErr != nil { diff --git a/chains/solana/utils/timelock/timelock_operations.go b/chains/solana/utils/timelock/timelock_operations.go index 1872055ac..479cacf49 100644 --- a/chains/solana/utils/timelock/timelock_operations.go +++ b/chains/solana/utils/timelock/timelock_operations.go @@ -25,6 +25,7 @@ type Operation struct { Salt [32]byte // random salt for the operation Delay uint64 // delay in seconds instructions []Instruction // instruction data slice, use Add method to add instructions and accounts + IsBypasserOp bool // is this operation for bypasser_execute_batch } // add instruction and required accounts to operation @@ -37,9 +38,6 @@ func (op *Operation) AddInstruction(ix solana.Instruction, additionalPrograms [] for i, acc := range ix.Accounts() { accounts[i] = *acc - // for debugging - // fmt.Printf("Account %d: %s (signer: %v, writable: %v)\n", - // i, acc.PublicKey, acc.IsSigner, acc.IsWritable) } op.instructions = append(op.instructions, Instruction{ @@ -77,7 +75,6 @@ func (op *Operation) RemainingAccounts() []*solana.AccountMeta { existing.IsWritable = existing.IsWritable || acc.IsWritable } else { accCopy := acc - // todo: maybe keep it as it is and override on chain(in case if we gonna calc root from this) accCopy.IsSigner = false // force false for CPI accountMap[key] = &accCopy } @@ -102,6 +99,9 @@ func (op *Operation) OperationID() [32]byte { func (op *Operation) OperationPDA() solana.PublicKey { id := op.OperationID() + if op.IsBypasserOp { + return GetBypasserOperationPDA(op.TimelockID, id) + } return GetOperationPDA(op.TimelockID, id) }