diff --git a/docs/docs/key-concepts/chain-metadata.md b/docs/docs/key-concepts/chain-metadata.md index 0e488af8..0925ba4b 100644 --- a/docs/docs/key-concepts/chain-metadata.md +++ b/docs/docs/key-concepts/chain-metadata.md @@ -27,3 +27,44 @@ The starting operation count, typically used for parallel signing processes. **mcmAddress** string
The MCM contract address that will process this proposal on the respective chain. + +--- + +**additionalFields** object _optional_
+Chain-family-specific fields encoded as JSON. Structure depends on the chain family (see below). + +### Solana Additional Fields + +Solana chain metadata uses `additionalFields` for the Timelock role access-controller accounts and, for bypass proposals, the execute fee payer. + +| Field | Required | When used | +| --- | --- | --- | +| `proposerRoleAccessController` | yes | schedule conversion | +| `cancellerRoleAccessController` | yes | cancel conversion | +| `bypasserRoleAccessController` | yes | bypass conversion | +| `executePayer` | no | bypass only — account that pays (and therefore signs) the outer MCM execute transaction | + +Example Solana `chainMetadata` entry: + +```json +"5013781088424303360": { + "startingOpCount": 0, + "mcmAddress": ".", + "additionalFields": { + "proposerRoleAccessController": "...", + "cancellerRoleAccessController": "...", + "bypasserRoleAccessController": "...", + "executePayer": "" + } +} +``` + +#### `executePayer` + +When the execute payer also appears in a bypass operation's `remaining_accounts` (for example as a BPF upgrade spill / close recipient), the Solana runtime always presents the fee payer as `IsSigner=true` at execution time. Off-chain conversion otherwise defaults remaining accounts to non-signer. Without recording `executePayer` in chain metadata, the Merkle leaf hashed off-chain does not match on-chain proof verification and execution fails with `ProofCannotBeVerified`. + +**When to set it:** Solana **bypass** proposals where the fee-payer pubkey is listed as a writable remaining account. Omit for schedule/cancel; the converter ignores `executePayer` for non-bypass actions. + +**Go helper:** `AdditionalFieldsMetadata.WithExecutePayer(pk)` in [`sdk/solana/chain_metadata.go`](https://github.com/smartcontractkit/mcms/blob/main/sdk/solana/chain_metadata.go). + +**Reference scenario:** [`e2e/tests/solana/timelock_bypass_payer_collision.go`](https://github.com/smartcontractkit/mcms/blob/main/e2e/tests/solana/timelock_bypass_payer_collision.go). diff --git a/docs/docs/key-concepts/timelock-proposal.md b/docs/docs/key-concepts/timelock-proposal.md index eeec6df1..5b0624b5 100644 --- a/docs/docs/key-concepts/timelock-proposal.md +++ b/docs/docs/key-concepts/timelock-proposal.md @@ -114,7 +114,7 @@ A Unix timestamp that specifies the proposal's expiration. If the proposal is no Specifies the high-level action for the proposal. Can be one of: - `schedule`: Sets up transactions to execute after a delay. - `cancel`: Cancels previously scheduled transactions. -- `bypass`: Directly executes transactions, skipping the timelock. +- `bypass`: Directly executes transactions, skipping the timelock. For Solana bypass proposals, if the execute fee payer also appears in a batch op's remaining accounts, set `executePayer` in that chain's `additionalFields` so Merkle proof verification succeeds. See [Chain Metadata — Solana Additional Fields](./chain-metadata.md#solana-additional-fields). --- diff --git a/docs/docs/usage/building-proposals.md b/docs/docs/usage/building-proposals.md index 957c794e..eb69d4d2 100644 --- a/docs/docs/usage/building-proposals.md +++ b/docs/docs/usage/building-proposals.md @@ -349,6 +349,9 @@ builder.AddOperation(types.Operation{ChainSelector: selector, Transaction: tx}) ``` +When building Solana **timelock bypass** proposals programmatically, if the execute fee payer appears in remaining accounts, set `executePayer` on that chain's metadata (for example via `AdditionalFieldsMetadata.WithExecutePayer`) so conversion hashes the Merkle leaf with `IsSigner=true`. See [Chain Metadata — Solana Additional Fields](../key-concepts/chain-metadata.md#solana-additional-fields). + + ### Aptos Operations Use the `aptos.NewTransaction` helper to build an Aptos specific transaction. diff --git a/e2e/tests/solana/timelock_bypass_payer_collision.go b/e2e/tests/solana/timelock_bypass_payer_collision.go new file mode 100644 index 00000000..1d6e3670 --- /dev/null +++ b/e2e/tests/solana/timelock_bypass_payer_collision.go @@ -0,0 +1,243 @@ +//go:build e2e + +package solanae2e + +import ( + "context" + "encoding/json" + "time" + + "github.com/ethereum/go-ethereum/common" + + "github.com/gagliardetto/solana-go" + "github.com/gagliardetto/solana-go/programs/system" + "github.com/gagliardetto/solana-go/rpc" + + "github.com/smartcontractkit/chainlink-ccip/chains/solana/gobindings/v0_1_1/timelock" + + "github.com/smartcontractkit/mcms" + e2eutils "github.com/smartcontractkit/mcms/e2e/utils/solana" + "github.com/smartcontractkit/mcms/sdk" + solanasdk "github.com/smartcontractkit/mcms/sdk/solana" + "github.com/smartcontractkit/mcms/types" +) + +var ( + testPDASeedBypassPayerWith = [32]byte{'t', 'e', 's', 't', '-', 'b', 'y', 'p', 'a', 's', 's', '-', 'p', 'a', 'y', 'e', 'r', '-', 'w'} + testPDASeedBypassPayerWithout = [32]byte{'t', 'e', 's', 't', '-', 'b', 'y', 'p', 'a', 's', 's', '-', 'p', 'a', 'y', 'e', 'r', '-', 'n'} +) + +const bypassPayerTransferLamports = 1_000_000 // 0.001 SOL + +// TestBypassExecutePayerInRemainingAccounts covers the Solana bypass failure +// where the execute payer (the deployer key) also appears in the +// BypasserExecuteBatch op's remaining_accounts. +// +// Real-world shape: a BPF-loader `upgrade` instruction lists the deployer as the +// spill/close recipient — a writable, non-signer account. Off-chain the Solana +// converter forces every remaining account to IsSigner=false before computing +// the Merkle root, so the deployer is hashed with IsSigner=false. At execution +// time the same deployer key is the outer transaction fee payer, so the Solana +// runtime presents it to the MCM program as IsSigner=true. The MCM program +// rebuilds the Merkle leaf from the runtime account infos, hashes IsSigner=true, +// and the one-bit mismatch invalidates the proof -> ProofCannotBeVerified. +// +// This test uses a system.Transfer whose recipient is the deployer/executor +// wallet to reproduce the identical one-bit collision without deploying an +// upgradeable program + buffer. +// +// - "with execute payer in metadata": the proposal's Solana chain metadata records +// the executor as executePayer, so the converter marks that account IsSigner=true +// before the root is computed and the bypass executes cleanly. +// - "without execute payer in metadata": the same proposal converted without the +// field still fails with ProofCannotBeVerified, documenting the bug and guarding +// against the fix silently becoming a no-op. +func (s *TestSuite) TestBypassExecutePayerInRemainingAccounts() { + s.Run("with execute payer in metadata: bypass succeeds", func() { + s.runBypassPayerCollision(testPDASeedBypassPayerWith, true) + }) + s.Run("without execute payer in metadata: proof fails", func() { + s.runBypassPayerCollision(testPDASeedBypassPayerWithout, false) + }) +} + +// runBypassPayerCollision drives the full bypass flow (convert -> set config -> +// sign -> set root -> execute) for a batch whose inner instruction sends +// lamports to the executor wallet. When setExecutePayerInMetadata is true, the +// executor is recorded in the proposal's Solana chain metadata so the converter +// marks it as a signer and the bypass execute succeeds; otherwise the final op +// fails with ProofCannotBeVerified. +func (s *TestSuite) runBypassPayerCollision(seed [32]byte, setExecutePayerInMetadata bool) { + // --- arrange --- + ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) + s.T().Cleanup(cancel) + + // wallet is the deployer key: MCM executor / outer transaction fee payer. + wallet, err := solana.PrivateKeyFromBase58(privateKey) + s.Require().NoError(err) + + s.SetupMCM(seed) + s.SetupTimelock(seed, 1*time.Second) + + mcmSignerPDA, err := solanasdk.FindSignerPDA(s.MCMProgramID, seed) + s.Require().NoError(err) + // The MCM signer PDA drives the bypass instructions, so it must hold the + // bypasser role. + s.AssignRoleToAccounts(ctx, seed, wallet, []solana.PublicKey{mcmSignerPDA}, timelock.Bypasser_Role) + + timelockSignerPDA, err := solanasdk.FindTimelockSignerPDA(s.TimelockProgramID, seed) + s.Require().NoError(err) + + // Fund the timelock signer PDA (transfer source, signs via CPI) and the mcm + // signer PDA. + e2eutils.FundAccounts(s.T(), []solana.PublicKey{mcmSignerPDA, timelockSignerPDA}, 1, s.SolanaClient) + + mcmAddress := solanasdk.ContractAddress(s.MCMProgramID, seed) + timelockAddress := solanasdk.ContractAddress(s.TimelockProgramID, seed) + + // --- inner "spill-like" instruction --- + // Transfer lamports from the timelock signer PDA to the deployer wallet. + // The recipient (wallet) is a writable, non-signer account: exactly the + // role a BPF upgrade spill account plays in production. + transferIx, err := system.NewTransferInstruction(bypassPayerTransferLamports, timelockSignerPDA, wallet.PublicKey()). + ValidateAndBuild() + s.Require().NoError(err) + + transferTx, err := solanasdk.NewTransactionFromInstruction(transferIx, "System", + []string{"bypass-payer-collision"}) + s.Require().NoError(err) + + batchOp := types.BatchOperation{ + ChainSelector: s.ChainSelector, + Transactions: []types.Transaction{transferTx}, + } + + // --- chain metadata --- + opCount, err := solanasdk.NewInspector(s.SolanaClient).GetOpCount(ctx, mcmAddress) + s.Require().NoError(err) + metadata, err := solanasdk.NewChainMetadata(opCount, s.MCMProgramID, seed, + s.Roles[timelock.Proposer_Role].AccessController.PublicKey(), + s.Roles[timelock.Canceller_Role].AccessController.PublicKey(), + s.Roles[timelock.Bypasser_Role].AccessController.PublicKey()) + s.Require().NoError(err) + if setExecutePayerInMetadata { + var additionalFields solanasdk.AdditionalFieldsMetadata + s.Require().NoError(json.Unmarshal(metadata.AdditionalFields, &additionalFields)) + additionalFields = additionalFields.WithExecutePayer(wallet.PublicKey()) + metadata.AdditionalFields, err = json.Marshal(additionalFields) + s.Require().NoError(err) + } + + // --- bypass proposal --- + timelockProposal, err := mcms.NewTimelockProposalBuilder(). + SetVersion("v1"). + SetValidUntil(2051222400). // 2035-01-01T00:00:00 UTC + SetDescription("bypass proposal: executor payer appears in remaining_accounts"). + SetOverridePreviousRoot(true). + SetDelay(types.NewDuration(1*time.Second)). + SetAction(types.TimelockActionBypass). + AddTimelockAddress(s.ChainSelector, timelockAddress). + AddChainMetadata(s.ChainSelector, metadata). + AddOperation(batchOp). + Build() + s.Require().NoError(err) + + converters := map[types.ChainSelector]sdk.TimelockConverter{ + s.ChainSelector: solanasdk.TimelockConverter{}, + } + + mcmsProposal, _, err := timelockProposal.Convert(ctx, converters) + s.Require().NoError(err) + + // The executor wallet lands in the final BypasserExecuteBatch op as a + // writable remaining account. Its IsSigner flag must reflect whether the + // execute payer was recorded in chain metadata. + s.assertExecutorSignerBit(mcmsProposal, wallet.PublicKey(), setExecutePayerInMetadata) + + // --- set config + sign + set root --- + signerEVMAccount := NewEVMTestAccount(s.T()) + mcmConfig := types.Config{Quorum: 1, Signers: []common.Address{signerEVMAccount.Address}} + configurer := solanasdk.NewConfigurer(s.SolanaClient, wallet, s.ChainSelector) + _, err = configurer.SetConfig(ctx, mcmAddress, &mcmConfig, true) + s.Require().NoError(err) + + inspectors := map[types.ChainSelector]sdk.Inspector{s.ChainSelector: solanasdk.NewInspector(s.SolanaClient)} + signable, err := mcms.NewSignable(&mcmsProposal, inspectors) + s.Require().NoError(err) + _, err = signable.SignAndAppend(mcms.NewPrivateKeySigner(signerEVMAccount.PrivateKey)) + s.Require().NoError(err) + + encoders, err := mcmsProposal.GetEncoders() //nolint:contextcheck,nolintlint //OPT-400 + s.Require().NoError(err) + encoder := encoders[s.ChainSelector].(*solanasdk.Encoder) + executors := map[types.ChainSelector]sdk.Executor{ + s.ChainSelector: solanasdk.NewExecutor(encoder, s.SolanaClient, wallet), + } + executable, err := mcms.NewExecutable(&mcmsProposal, executors) //nolint:contextcheck,nolintlint //OPT-400 + s.Require().NoError(err) + + _, err = executable.SetRoot(ctx, s.ChainSelector) + s.Require().NoError(err) + + // --- act + assert --- + // The set-up ops (init/append/finalize bypasser operation) never include the + // executor key, so their proofs verify regardless. Only the final + // BypasserExecuteBatch op carries the executor key in its remaining accounts. + lastOp := len(mcmsProposal.Operations) - 1 + s.Require().Positive(lastOp, "expected multiple bypass ops") + + balanceBefore := s.lamports(ctx, timelockSignerPDA) + + // Execute setup ops (init/append/finalize); they don't carry the executor key + // in their accounts so their proofs verify regardless of execute payer metadata. + for i := range lastOp { + _, err = executable.Execute(ctx, i) + s.Require().NoError(err, "unexpected failure on setup op %d", i) + } + + // Execute the final BypasserExecuteBatch op — the one whose remaining_accounts + // include the executor wallet, causing the signer-bit collision. + _, execErr := executable.Execute(ctx, lastOp) + if setExecutePayerInMetadata { + s.Require().NoError(execErr, "BypasserExecuteBatch should succeed once the execute payer is a signer") + } else { + s.Require().Error(execErr, "expected BypasserExecuteBatch to fail due to execute-payer signer collision") + s.Require().ErrorContains(execErr, "ProofCannotBeVerified") + } + + if setExecutePayerInMetadata { + // The inner transfer actually moved lamports out of the timelock signer PDA. + balanceAfter := s.lamports(ctx, timelockSignerPDA) + s.Require().Equal(balanceBefore-bypassPayerTransferLamports, balanceAfter, + "timelock signer PDA should have sent exactly the transfer amount") + } +} + +// assertExecutorSignerBit checks the executor key appears in the last converted +// op (BypasserExecuteBatch) as a writable remaining account with the expected +// IsSigner flag. +func (s *TestSuite) assertExecutorSignerBit(proposal mcms.Proposal, executor solana.PublicKey, wantSigner bool) { + s.Require().NotEmpty(proposal.Operations) + lastOp := proposal.Operations[len(proposal.Operations)-1] + + var fields solanasdk.AdditionalFields + s.Require().NoError(json.Unmarshal(lastOp.Transaction.AdditionalFields, &fields)) + + found := false + for _, acc := range fields.Accounts { + if acc.PublicKey.Equals(executor) { + found = true + s.Require().Equal(wantSigner, acc.IsSigner, "executor IsSigner flag mismatch in converted bypass op") + s.Require().True(acc.IsWritable, "executor (transfer recipient) should be writable") + } + } + s.Require().True(found, "executor key must appear in the BypasserExecuteBatch remaining accounts") +} + +// lamports returns the current lamport balance of the given account. +func (s *TestSuite) lamports(ctx context.Context, account solana.PublicKey) uint64 { + res, err := s.SolanaClient.GetBalance(ctx, account, rpc.CommitmentConfirmed) + s.Require().NoError(err) + + return res.Value +} diff --git a/sdk/solana/chain_metadata.go b/sdk/solana/chain_metadata.go index 6a29075e..1a4b3284 100644 --- a/sdk/solana/chain_metadata.go +++ b/sdk/solana/chain_metadata.go @@ -17,6 +17,19 @@ type AdditionalFieldsMetadata struct { ProposerRoleAccessController solana.PublicKey `json:"proposerRoleAccessController" validate:"required"` CancellerRoleAccessController solana.PublicKey `json:"cancellerRoleAccessController" validate:"required"` BypasserRoleAccessController solana.PublicKey `json:"bypasserRoleAccessController" validate:"required"` + // ExecutePayer is the optional outer MCM execute fee payer (bypass only). + ExecutePayer *solana.PublicKey `json:"executePayer,omitempty"` +} + +// WithExecutePayer returns a copy of f with ExecutePayer set to pk. +func (f AdditionalFieldsMetadata) WithExecutePayer(pk solana.PublicKey) AdditionalFieldsMetadata { + f.ExecutePayer = &pk + return f +} + +// HasExecutePayer reports whether ExecutePayer is set to a non-zero public key. +func (f AdditionalFieldsMetadata) HasExecutePayer() bool { + return f.ExecutePayer != nil && !f.ExecutePayer.IsZero() } func (f AdditionalFieldsMetadata) Validate() error { diff --git a/sdk/solana/chain_metadata_test.go b/sdk/solana/chain_metadata_test.go index 819e4fb2..c24daa6c 100644 --- a/sdk/solana/chain_metadata_test.go +++ b/sdk/solana/chain_metadata_test.go @@ -1,341 +1,163 @@ package solana import ( - "context" "encoding/json" "errors" "testing" - "github.com/stretchr/testify/require" - - "gotest.tools/v3/assert" - - "github.com/google/go-cmp/cmp" - - "github.com/smartcontractkit/mcms/types" - "github.com/gagliardetto/solana-go" "github.com/gagliardetto/solana-go/rpc" + "github.com/stretchr/testify/require" "github.com/smartcontractkit/chainlink-ccip/chains/solana/gobindings/v0_1_1/timelock" "github.com/smartcontractkit/mcms/sdk/solana/mocks" + "github.com/smartcontractkit/mcms/types" ) +func validAdditionalFields(t *testing.T) AdditionalFieldsMetadata { + t.Helper() + return AdditionalFieldsMetadata{ + ProposerRoleAccessController: solana.NewWallet().PublicKey(), + CancellerRoleAccessController: solana.NewWallet().PublicKey(), + BypasserRoleAccessController: solana.NewWallet().PublicKey(), + } +} + func TestNewChainMetadataFromTimelock(t *testing.T) { t.Parallel() - type params struct { - startingOpCount uint64 - mcmProgramID solana.PublicKey - mcmInstanceSeed PDASeed - timelock solana.PublicKey - timelockSeed PDASeed - } - programID := solana.NewWallet().PublicKey() timelockProgramID := solana.NewWallet().PublicKey() - MCMSeed := PDASeed([32]byte{1, 2, 3, 4}) + mcmSeed := PDASeed([32]byte{1, 2, 3, 4}) timelockSeed := PDASeed([32]byte{1, 2, 3, 4}) configPDA, err := FindTimelockConfigPDA(timelockProgramID, timelockSeed) require.NoError(t, err) - tests := []struct { - name string - params params - setupMock func(mock *mocks.JSONRPCClient) - wantMetadata *types.ChainMetadata - wantErr error - }{ - { - name: "valid metadata", - params: params{ - startingOpCount: 100, - mcmProgramID: programID, - mcmInstanceSeed: MCMSeed, - timelock: timelockProgramID, - timelockSeed: timelockSeed, - }, - setupMock: func(mockJSONRPCClient *mocks.JSONRPCClient) { - mockGetAccountInfo(t, mockJSONRPCClient, configPDA, &timelock.Config{}, nil) - }, - wantMetadata: &types.ChainMetadata{ - StartingOpCount: 100, - MCMAddress: ContractAddress(programID, MCMSeed), - AdditionalFields: json.RawMessage(`{"proposerRoleAccessController":"11111111111111111111111111111111","cancellerRoleAccessController":"11111111111111111111111111111111","bypasserRoleAccessController":"11111111111111111111111111111111"}`), - }, - }, - { - name: "error rpc call", - params: params{ - startingOpCount: 100, - mcmProgramID: programID, - mcmInstanceSeed: MCMSeed, - timelock: timelockProgramID, - timelockSeed: timelockSeed, - }, - wantErr: errors.New("unable to read timelock config pda: rpc error"), - setupMock: func(mockJSONRPCClient *mocks.JSONRPCClient) { - err := errors.New("rpc error") - mockGetAccountInfo(t, mockJSONRPCClient, configPDA, &timelock.Config{}, err) - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + newClient := func(t *testing.T, rpcErr error) *rpc.Client { + t.Helper() + jsonRPC := mocks.NewJSONRPCClient(t) + mockGetAccountInfo(t, jsonRPC, configPDA, &timelock.Config{}, rpcErr) - jsonRPC := mocks.NewJSONRPCClient(t) - tt.setupMock(jsonRPC) - client := rpc.NewWithCustomRPCClient(jsonRPC) - metadata, err := NewChainMetadataFromTimelock( - context.Background(), - client, - tt.params.startingOpCount, - tt.params.mcmProgramID, - tt.params.mcmInstanceSeed, - tt.params.timelock, - tt.params.timelockSeed) - if tt.wantErr == nil { - require.NoError(t, err, "expected no error but got one") - require.Empty(t, cmp.Diff(tt.wantMetadata, &metadata)) - } else { - // Assert the error message matches the expected error. - require.NotNil(t, metadata) - require.EqualError(t, err, tt.wantErr.Error()) - } - }) + return rpc.NewWithCustomRPCClient(jsonRPC) } + + t.Run("returns metadata from timelock config", func(t *testing.T) { + t.Parallel() + metadata, err := NewChainMetadataFromTimelock( + t.Context(), newClient(t, nil), 100, programID, mcmSeed, timelockProgramID, timelockSeed) + require.NoError(t, err) + require.Equal(t, uint64(100), metadata.StartingOpCount) + require.Equal(t, ContractAddress(programID, mcmSeed), metadata.MCMAddress) + }) + + t.Run("wraps RPC errors", func(t *testing.T) { + t.Parallel() + _, err := NewChainMetadataFromTimelock( + t.Context(), newClient(t, errors.New("rpc error")), 100, programID, mcmSeed, timelockProgramID, timelockSeed) + require.EqualError(t, err, "unable to read timelock config pda: rpc error") + }) } -func TestAdditionalFieldsMetadata_Validate(t *testing.T) { +func TestAdditionalFieldsMetadata_ExecutePayer(t *testing.T) { t.Parallel() - // Create valid public keys for testing. - validPK1, err := solana.NewRandomPrivateKey() - require.NoError(t, err) - validPK2, err := solana.NewRandomPrivateKey() - require.NoError(t, err) - validPK3, err := solana.NewRandomPrivateKey() - require.NoError(t, err) - zeroPK := solana.PublicKey{} // zero value public key - - tests := []struct { - name string - fields AdditionalFieldsMetadata - expectedErr error - }{ - { - name: "all valid keys", - fields: AdditionalFieldsMetadata{ - ProposerRoleAccessController: validPK1.PublicKey(), - CancellerRoleAccessController: validPK2.PublicKey(), - BypasserRoleAccessController: validPK3.PublicKey(), - }, - expectedErr: nil, - }, - { - name: "zero proposer key", - fields: AdditionalFieldsMetadata{ - ProposerRoleAccessController: zeroPK, - CancellerRoleAccessController: validPK2.PublicKey(), - BypasserRoleAccessController: validPK3.PublicKey(), - }, - expectedErr: errors.New("Key: 'AdditionalFieldsMetadata.ProposerRoleAccessController' Error:Field validation for 'ProposerRoleAccessController' failed on the 'required' tag"), - }, - { - name: "zero canceller key", - fields: AdditionalFieldsMetadata{ - ProposerRoleAccessController: validPK1.PublicKey(), - CancellerRoleAccessController: zeroPK, - BypasserRoleAccessController: validPK3.PublicKey(), - }, - expectedErr: errors.New("Key: 'AdditionalFieldsMetadata.CancellerRoleAccessController' Error:Field validation for 'CancellerRoleAccessController' failed on the 'required' tag"), - }, - { - name: "zero bypasser key", - fields: AdditionalFieldsMetadata{ - ProposerRoleAccessController: validPK1.PublicKey(), - CancellerRoleAccessController: validPK2.PublicKey(), - BypasserRoleAccessController: zeroPK, - }, - expectedErr: errors.New("Key: 'AdditionalFieldsMetadata.BypasserRoleAccessController' Error:Field validation for 'BypasserRoleAccessController' failed on the 'required' tag"), - }, - } + base := validAdditionalFields(t) + payer := solana.NewWallet().PublicKey() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + t.Run("WithExecutePayer returns copy without mutating original", func(t *testing.T) { + t.Parallel() + updated := base.WithExecutePayer(payer) + require.Nil(t, base.ExecutePayer) + require.True(t, updated.ExecutePayer.Equals(payer)) + require.True(t, updated.ProposerRoleAccessController.Equals(base.ProposerRoleAccessController)) + }) - err := tt.fields.Validate() - if tt.expectedErr == nil { - require.NoError(t, err, "expected no error but got one") - } else { - // Assert the error message matches the expected error. - require.EqualError(t, err, tt.expectedErr.Error()) - } - }) - } -} + t.Run("HasExecutePayer is false for nil and zero key, true when set", func(t *testing.T) { + t.Parallel() + require.False(t, base.HasExecutePayer()) -func TestValidateChainMetadata(t *testing.T) { - t.Parallel() + zero := solana.PublicKey{} + withZero := base + withZero.ExecutePayer = &zero + require.False(t, withZero.HasExecutePayer()) - // Create some public keys for testing. - zeroPK := solana.PublicKey{} // zero value public key + require.True(t, base.WithExecutePayer(payer).HasExecutePayer()) + }) - // Valid additional fields. - validFields := AdditionalFieldsMetadata{ - ProposerRoleAccessController: solana.NewWallet().PublicKey(), - CancellerRoleAccessController: solana.NewWallet().PublicKey(), - BypasserRoleAccessController: solana.NewWallet().PublicKey(), - } - validJSON, err := json.Marshal(validFields) - require.NoError(t, err) + t.Run("JSON round-trips executePayer, omits when nil", func(t *testing.T) { + t.Parallel() - // Missing required field. - // Here we omit CancellerRoleAccessController so that field remains at its zero value. - // Using an inline struct with only two fields. - missingField := struct { - ProposerRoleAccessController solana.PublicKey `json:"proposerRoleAccessController"` - BypasserRoleAccessController solana.PublicKey `json:"bypasserRoleAccessController"` - }{ - ProposerRoleAccessController: validFields.ProposerRoleAccessController, - BypasserRoleAccessController: validFields.BypasserRoleAccessController, - } - missingFieldJSON, err := json.Marshal(missingField) - require.NoError(t, err) + raw, err := json.Marshal(base) + require.NoError(t, err) + require.NotContains(t, string(raw), "executePayer") - // Zero value field: Proposer is zero. - zeroField := AdditionalFieldsMetadata{ - ProposerRoleAccessController: zeroPK, - CancellerRoleAccessController: validFields.CancellerRoleAccessController, - BypasserRoleAccessController: validFields.BypasserRoleAccessController, - } - zeroFieldJSON, err := json.Marshal(zeroField) - require.NoError(t, err) + raw, err = json.Marshal(base.WithExecutePayer(payer)) + require.NoError(t, err) - tests := []struct { - name string - metadata types.ChainMetadata - expectedErr bool - }{ - { - name: "valid additional fields", - metadata: types.ChainMetadata{ - AdditionalFields: validJSON, - }, - expectedErr: false, - }, - { - name: "invalid JSON", - metadata: types.ChainMetadata{ - AdditionalFields: []byte("not a json"), - }, - expectedErr: true, - }, - { - name: "missing required field", - metadata: types.ChainMetadata{ - AdditionalFields: missingFieldJSON, - }, - expectedErr: true, - }, - { - name: "zero value in one field", - metadata: types.ChainMetadata{ - AdditionalFields: zeroFieldJSON, - }, - expectedErr: true, - }, - } + var roundTrip AdditionalFieldsMetadata + require.NoError(t, json.Unmarshal(raw, &roundTrip)) + require.True(t, roundTrip.ExecutePayer.Equals(payer)) + }) +} + +func TestAdditionalFieldsMetadata_Validate(t *testing.T) { + t.Parallel() + + require.NoError(t, validAdditionalFields(t).Validate(), "all valid keys") + require.NoError(t, validAdditionalFields(t).WithExecutePayer(solana.NewWallet().PublicKey()).Validate(), "with execute payer") - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, field := range []string{"ProposerRoleAccessController", "CancellerRoleAccessController", "BypasserRoleAccessController"} { + t.Run("rejects zero "+field, func(t *testing.T) { t.Parallel() - err := ValidateChainMetadata(tt.metadata) - if tt.expectedErr { - require.Error(t, err, "expected an error for test case: %s", tt.name) - } else { - require.NoError(t, err, "expected no error for test case: %s", tt.name) + fields := validAdditionalFields(t) + switch field { + case "ProposerRoleAccessController": + fields.ProposerRoleAccessController = solana.PublicKey{} + case "CancellerRoleAccessController": + fields.CancellerRoleAccessController = solana.PublicKey{} + case "BypasserRoleAccessController": + fields.BypasserRoleAccessController = solana.PublicKey{} } + require.ErrorContains(t, fields.Validate(), field) }) } } -func TestNewSolanaChainMetadata(t *testing.T) { +func TestValidateChainMetadata(t *testing.T) { t.Parallel() - // Create sample public keys. - mcmProgramID, err := solana.NewRandomPrivateKey() + raw, err := json.Marshal(validAdditionalFields(t)) require.NoError(t, err) + require.NoError(t, ValidateChainMetadata(types.ChainMetadata{AdditionalFields: raw}), "valid fields") - proposerKey, err := solana.NewRandomPrivateKey() - require.NoError(t, err) + require.ErrorContains(t, ValidateChainMetadata(types.ChainMetadata{AdditionalFields: []byte("bad")}), "unable to unmarshal") - cancellerKey, err := solana.NewRandomPrivateKey() + invalid := validAdditionalFields(t) + invalid.ProposerRoleAccessController = solana.PublicKey{} + raw, err = json.Marshal(invalid) require.NoError(t, err) + require.ErrorContains(t, ValidateChainMetadata(types.ChainMetadata{AdditionalFields: raw}), "additional fields are invalid") +} - bypasserKey, err := solana.NewRandomPrivateKey() - require.NoError(t, err) - - tests := []struct { - name string - startingOpCount uint64 - mcmProgramID solana.PublicKey - mcmInstanceSeed PDASeed - proposerKey solana.PublicKey - cancellerKey solana.PublicKey - bypasserKey solana.PublicKey - wantErr string - }{ - { - name: "valid metadata", - startingOpCount: 100, - mcmProgramID: mcmProgramID.PublicKey(), - mcmInstanceSeed: PDASeed([32]byte{1, 2, 3, 4}), - proposerKey: proposerKey.PublicKey(), - cancellerKey: cancellerKey.PublicKey(), - bypasserKey: bypasserKey.PublicKey(), - }, - { - name: "invalid metadata", - startingOpCount: 100, - mcmProgramID: solana.PublicKey{}, - mcmInstanceSeed: PDASeed([32]byte{1, 2, 3, 4}), - proposerKey: proposerKey.PublicKey(), - cancellerKey: cancellerKey.PublicKey(), - bypasserKey: bypasserKey.PublicKey(), - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - metadata, err := NewChainMetadata(tc.startingOpCount, tc.mcmProgramID, tc.mcmInstanceSeed, tc.proposerKey, tc.cancellerKey, tc.bypasserKey) - if tc.wantErr != "" { - require.EqualError(t, err, tc.wantErr) - return - } - require.NoError(t, err) - - assert.Equal(t, tc.startingOpCount, metadata.StartingOpCount) - - expectedMCMAddress := ContractAddress(tc.mcmProgramID, tc.mcmInstanceSeed) - assert.Equal(t, expectedMCMAddress, metadata.MCMAddress) +func TestNewChainMetadata(t *testing.T) { + t.Parallel() - var additionalFields AdditionalFieldsMetadata - err = json.Unmarshal(metadata.AdditionalFields, &additionalFields) - require.NoError(t, err) + proposer := solana.NewWallet().PublicKey() + canceller := solana.NewWallet().PublicKey() + bypasser := solana.NewWallet().PublicKey() + programID := solana.NewWallet().PublicKey() + seed := PDASeed([32]byte{1, 2, 3, 4}) - expectedAdditionalFields := AdditionalFieldsMetadata{ - ProposerRoleAccessController: tc.proposerKey, - CancellerRoleAccessController: tc.cancellerKey, - BypasserRoleAccessController: tc.bypasserKey, - } - assert.Equal(t, expectedAdditionalFields, additionalFields) - }) - } + metadata, err := NewChainMetadata(100, programID, seed, proposer, canceller, bypasser) + require.NoError(t, err) + require.Equal(t, uint64(100), metadata.StartingOpCount) + require.Equal(t, ContractAddress(programID, seed), metadata.MCMAddress) + + var additional AdditionalFieldsMetadata + require.NoError(t, json.Unmarshal(metadata.AdditionalFields, &additional)) + require.True(t, additional.ProposerRoleAccessController.Equals(proposer)) + require.True(t, additional.CancellerRoleAccessController.Equals(canceller)) + require.True(t, additional.BypasserRoleAccessController.Equals(bypasser)) } diff --git a/sdk/solana/encoder.go b/sdk/solana/encoder.go index b1ee1d75..8abe981b 100644 --- a/sdk/solana/encoder.go +++ b/sdk/solana/encoder.go @@ -3,7 +3,6 @@ package solana import ( "bytes" "encoding/binary" - "encoding/json" "fmt" "github.com/ethereum/go-ethereum/common" @@ -61,12 +60,9 @@ func (e *Encoder) HashOperation( return common.Hash{}, fmt.Errorf("unable to prase program id from To field: %w", err) } - // Parse Additional fields to get the ix accounts - var additionalFields AdditionalFields - if op.Transaction.AdditionalFields != nil { - if err = json.Unmarshal(op.Transaction.AdditionalFields, &additionalFields); err != nil { - return common.Hash{}, fmt.Errorf("unable to unmarshal additional fields: %w", err) - } + additionalFields, err := ParseAdditionalFields(op.Transaction.AdditionalFields) + if err != nil { + return common.Hash{}, err } buffers := [][]byte{ diff --git a/sdk/solana/executor.go b/sdk/solana/executor.go index d10dcde1..6beb7855 100644 --- a/sdk/solana/executor.go +++ b/sdk/solana/executor.go @@ -2,7 +2,6 @@ package solana import ( "context" - "encoding/json" "fmt" "math" "regexp" @@ -90,10 +89,9 @@ func (e *Executor) ExecuteOperation( return types.TransactionResult{}, err } - // Unmarshal the AdditionalFields from the operation - var additionalFields AdditionalFields - if err = json.Unmarshal(op.Transaction.AdditionalFields, &additionalFields); err != nil { - return types.TransactionResult{}, fmt.Errorf("unable to unmarshal additional fields: %w", err) + additionalFields, err := ParseAdditionalFields(op.Transaction.AdditionalFields) + if err != nil { + return types.TransactionResult{}, err } toProgramID, err := ParseProgramID(op.Transaction.To) if err != nil { diff --git a/sdk/solana/simulator.go b/sdk/solana/simulator.go index 72d78fb7..52bc7e45 100644 --- a/sdk/solana/simulator.go +++ b/sdk/solana/simulator.go @@ -2,7 +2,6 @@ package solana import ( "context" - "encoding/json" "fmt" "time" @@ -48,9 +47,9 @@ func (s *Simulator) SimulateSetRoot( func (s *Simulator) SimulateOperation( ctx context.Context, metadata types.ChainMetadata, operation types.Operation, ) error { - var additionalFields AdditionalFields - if err := json.Unmarshal(operation.Transaction.AdditionalFields, &additionalFields); err != nil { - return fmt.Errorf("unable to unmarshal additional fields: %w", err) + additionalFields, err := ParseAdditionalFields(operation.Transaction.AdditionalFields) + if err != nil { + return err } toProgramID, err := ParseProgramID(operation.Transaction.To) diff --git a/sdk/solana/timelock_converter.go b/sdk/solana/timelock_converter.go index f9c8cf06..8de3d6ed 100644 --- a/sdk/solana/timelock_converter.go +++ b/sdk/solana/timelock_converter.go @@ -53,6 +53,12 @@ func (t TimelockConverter) ConvertBatchToChainOperations( bindings.SetProgramID(timelockProgramID) tags := getTagsFromBatchOperation(batchOp) + + var additionalFields AdditionalFieldsMetadata + if err = json.Unmarshal(metadata.AdditionalFields, &additionalFields); err != nil { + return []types.Operation{}, common.Hash{}, fmt.Errorf("unable to unmarshal solana-specific additional fields from chain metadata: %w", err) + } + instructionsData, err := getInstructionDataFromBatchOperation(batchOp) if err != nil { return []types.Operation{}, common.Hash{}, fmt.Errorf("unable to convert batch operation to solana instructions: %w", err) @@ -86,10 +92,7 @@ func (t TimelockConverter) ConvertBatchToChainOperations( if err != nil { return []types.Operation{}, common.Hash{}, fmt.Errorf("unable to find mcm signer address: %w", err) } - var additionalFields AdditionalFieldsMetadata - if err = json.Unmarshal(metadata.AdditionalFields, &additionalFields); err != nil { - return []types.Operation{}, common.Hash{}, fmt.Errorf("unable to unmarshal solana-specific additional fields from chain metada: %w", err) - } + // encode the data based on the operation var instructions []solana.Instruction switch action { @@ -101,13 +104,13 @@ func (t TimelockConverter) ConvertBatchToChainOperations( instructions, err = cancelInstructions(timelockPDASeed, operationID, additionalFields.CancellerRoleAccessController, operationPDA, configPDA, mcmSignerPDA) case types.TimelockActionBypass: - accounts, rerr := getAccountsFromBatchOperation(batchOp) - if rerr != nil { - return []types.Operation{}, common.Hash{}, fmt.Errorf("unable to get accounts from batch operation: %w", err) + bypassAccounts, bypassErr := bypassRemainingAccounts(batchOp, additionalFields) + if bypassErr != nil { + return []types.Operation{}, common.Hash{}, fmt.Errorf("unable to get accounts from batch operation: %w", bypassErr) } instructions, err = bypassInstructions(timelockPDASeed, operationID, additionalFields.BypasserRoleAccessController, operationBypasserPDA, configPDA, signerPDA, mcmSignerPDA, salt, uint32(len(batchOp.Transactions)), instructionsData, //nolint:gosec - accounts) + bypassAccounts) default: err = fmt.Errorf("invalid timelock operation: %s", string(action)) } @@ -198,12 +201,9 @@ func getInstructionDataFromBatchOperation(batchOp types.BatchOperation) ([]bindi return nil, fmt.Errorf("unable to parse program id from To field: %w", err) } - var additionalFields AdditionalFields - if len(tx.AdditionalFields) > 0 { - err = json.Unmarshal(tx.AdditionalFields, &additionalFields) - if err != nil { - return nil, fmt.Errorf("unable to unmarshal Solana additional fields: %w\n%v", err, string(tx.AdditionalFields)) - } + additionalFields, err := ParseAdditionalFields(tx.AdditionalFields) + if err != nil { + return nil, err } instructionsData = append(instructionsData, bindings.InstructionData{ @@ -225,12 +225,9 @@ func getAccountsFromBatchOperation(batchOp types.BatchOperation) ([]*solana.Acco } accounts = append(accounts, &solana.AccountMeta{PublicKey: toProgramID}) - var additionalFields AdditionalFields - if len(tx.AdditionalFields) > 0 { - err = json.Unmarshal(tx.AdditionalFields, &additionalFields) - if err != nil { - return nil, fmt.Errorf("unable to unmarshal additional fields: %w\n%v", err, string(tx.AdditionalFields)) - } + additionalFields, err := ParseAdditionalFields(tx.AdditionalFields) + if err != nil { + return nil, err } accounts = append(accounts, additionalFields.Accounts...) } @@ -253,6 +250,32 @@ func getAccountsFromBatchOperation(batchOp types.BatchOperation) ([]*solana.Acco return uniqueAccounts, nil } +// bypassRemainingAccounts collects remaining accounts for a bypass op and applies +// the execute-payer signer override when ExecutePayer is set in chain metadata. +func bypassRemainingAccounts( + batchOp types.BatchOperation, additionalFields AdditionalFieldsMetadata, +) ([]*solana.AccountMeta, error) { + accounts, err := getAccountsFromBatchOperation(batchOp) + if err != nil { + return nil, err + } + if additionalFields.HasExecutePayer() { + applyExecutePayerSignerOverride(accounts, *additionalFields.ExecutePayer) + } + + return accounts, nil +} + +// applyExecutePayerSignerOverride marks payer as IsSigner=true in accounts (in place). +func applyExecutePayerSignerOverride(accounts []*solana.AccountMeta, payer solana.PublicKey) { + for _, acc := range accounts { + if acc != nil && acc.PublicKey.Equals(payer) { + acc.IsSigner = true + return + } + } +} + func syncWritableAttribute(accounts []*solana.AccountMeta) []*solana.AccountMeta { writableAttrMap := map[solana.PublicKey]bool{} for _, account := range accounts { diff --git a/sdk/solana/timelock_converter_test.go b/sdk/solana/timelock_converter_test.go index 32e2864b..1ce28867 100644 --- a/sdk/solana/timelock_converter_test.go +++ b/sdk/solana/timelock_converter_test.go @@ -501,6 +501,353 @@ func TestTimelockConverter_ConvertBatchToChainOperations(t *testing.T) { } } +func TestTimelockConverter_ExecutePayerSignerOverride(t *testing.T) { + t.Parallel() + + timelockAddress := ContractAddress(testTimelockProgramID, testPDASeed) + mcmAddress := ContractAddress(testMCMProgramID, testPDASeed) + + proposerAC, err := solana.NewRandomPrivateKey() + require.NoError(t, err) + cancellerAC, err := solana.NewRandomPrivateKey() + require.NoError(t, err) + bypasserAC, err := solana.NewRandomPrivateKey() + require.NoError(t, err) + + payer, err := solana.NewRandomPrivateKey() + require.NoError(t, err) + + baseMetadata := AdditionalFieldsMetadata{ + ProposerRoleAccessController: proposerAC.PublicKey(), + CancellerRoleAccessController: cancellerAC.PublicKey(), + BypasserRoleAccessController: bypasserAC.PublicKey(), + } + metadataWithoutPayer := types.ChainMetadata{MCMAddress: mcmAddress} + metadataWithPayer := types.ChainMetadata{MCMAddress: mcmAddress} + metaBytes, err := json.Marshal(baseMetadata) + require.NoError(t, err) + metadataWithoutPayer.AdditionalFields = metaBytes + metaBytesWithPayer, err := json.Marshal(baseMetadata.WithExecutePayer(payer.PublicKey())) + require.NoError(t, err) + metadataWithPayer.AdditionalFields = metaBytesWithPayer + + // A batch op whose remaining accounts include the payer as a writable, + // non-signer account (mirroring a BPF spill / transfer recipient). + batchOp := func() types.BatchOperation { + return types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: "11111111111111111111111111111111", + Data: []byte{1, 2, 3, 4}, + AdditionalFields: toJSON(t, AdditionalFields{Accounts: []*solana.AccountMeta{ + {PublicKey: payer.PublicKey(), IsWritable: true}, + }}), + OperationMetadata: types.OperationMetadata{ContractType: "System", Tags: []string{"t"}}, + }}, + } + } + + convert := func(t *testing.T, metadata types.ChainMetadata, action types.TimelockAction) []types.Operation { + t.Helper() + ops, _, cerr := TimelockConverter{}.ConvertBatchToChainOperations(context.Background(), metadata, batchOp(), + timelockAddress, mcmAddress, types.NewDuration(time.Second), action, common.Hash{}, + common.HexToHash("0x01")) + require.NoError(t, cerr) + require.NotEmpty(t, ops) + + return ops + } + + // payerIsSigner reports the IsSigner flag of the payer account in the last + // converted op (BypasserExecuteBatch for a bypass), and whether it was found. + payerIsSigner := func(t *testing.T, ops []types.Operation) (isSigner, found bool) { + t.Helper() + last := ops[len(ops)-1] + var fields AdditionalFields + require.NoError(t, json.Unmarshal(last.Transaction.AdditionalFields, &fields)) + for _, acc := range fields.Accounts { + if acc.PublicKey.Equals(payer.PublicKey()) { + return acc.IsSigner, true + } + } + + return false, false + } + + t.Run("bypass, no execute payer in metadata: payer stays non-signer", func(t *testing.T) { + t.Parallel() + isSigner, found := payerIsSigner(t, convert(t, metadataWithoutPayer, types.TimelockActionBypass)) + require.True(t, found, "payer must appear in bypass remaining accounts") + require.False(t, isSigner) + }) + + t.Run("bypass, metadata with execute payer: payer becomes signer", func(t *testing.T) { + t.Parallel() + isSigner, found := payerIsSigner(t, convert(t, metadataWithPayer, types.TimelockActionBypass)) + require.True(t, found) + require.True(t, isSigner) + }) + + t.Run("bypass, metadata payer not in accounts: no-op", func(t *testing.T) { + t.Parallel() + other, oerr := solana.NewRandomPrivateKey() + require.NoError(t, oerr) + metaBytesOtherPayer, merr := json.Marshal(baseMetadata.WithExecutePayer(other.PublicKey())) + require.NoError(t, merr) + metadataOtherPayer := types.ChainMetadata{MCMAddress: mcmAddress, AdditionalFields: metaBytesOtherPayer} + isSigner, found := payerIsSigner(t, convert(t, metadataOtherPayer, types.TimelockActionBypass)) + require.True(t, found) + require.False(t, isSigner, "override must not touch accounts other than the configured payer") + }) + + t.Run("schedule, metadata with execute payer: unchanged", func(t *testing.T) { + t.Parallel() + withPayer := convert(t, metadataWithPayer, types.TimelockActionSchedule) + withoutPayer := convert(t, metadataWithoutPayer, types.TimelockActionSchedule) + require.Empty(t, cmp.Diff(withoutPayer, withPayer), "schedule conversion must ignore execute payers") + }) + + t.Run("bypass, zero execute payer pointer: treated as unset", func(t *testing.T) { + t.Parallel() + zero := solana.PublicKey{} + metaBytesZero, zerr := json.Marshal(AdditionalFieldsMetadata{ + ProposerRoleAccessController: proposerAC.PublicKey(), + CancellerRoleAccessController: cancellerAC.PublicKey(), + BypasserRoleAccessController: bypasserAC.PublicKey(), + ExecutePayer: &zero, + }) + require.NoError(t, zerr) + metadataZeroPayer := types.ChainMetadata{MCMAddress: mcmAddress, AdditionalFields: metaBytesZero} + isSigner, found := payerIsSigner(t, convert(t, metadataZeroPayer, types.TimelockActionBypass)) + require.True(t, found) + require.False(t, isSigner, "zero ExecutePayer must not trigger the signer override") + }) + + t.Run("bypass, nil remaining account: ConvertBatch returns wrapped error", func(t *testing.T) { + t.Parallel() + badBatch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: "11111111111111111111111111111111", + Data: []byte{1}, + AdditionalFields: []byte(`{"accounts":[null]}`), + }}, + } + _, _, cerr := TimelockConverter{}.ConvertBatchToChainOperations(context.Background(), metadataWithPayer, badBatch, + timelockAddress, mcmAddress, types.NewDuration(time.Second), types.TimelockActionBypass, common.Hash{}, + common.HexToHash("0x01")) + require.Error(t, cerr) + require.ErrorContains(t, cerr, "unable to convert batch operation to solana instructions") + require.ErrorContains(t, cerr, "nil account in batch operation additional fields") + }) + + t.Run("schedule, nil remaining account: ConvertBatch returns error", func(t *testing.T) { + t.Parallel() + badBatch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: "11111111111111111111111111111111", + Data: []byte{1}, + AdditionalFields: []byte(`{"accounts":[null]}`), + }}, + } + _, _, cerr := TimelockConverter{}.ConvertBatchToChainOperations(context.Background(), metadataWithPayer, badBatch, + timelockAddress, mcmAddress, types.NewDuration(time.Second), types.TimelockActionSchedule, common.Hash{}, + common.HexToHash("0x01")) + require.Error(t, cerr) + require.ErrorContains(t, cerr, "unable to convert batch operation to solana instructions") + require.ErrorContains(t, cerr, "nil account in batch operation additional fields") + }) + + t.Run("invalid chain metadata additional fields", func(t *testing.T) { + t.Parallel() + badMetadata := types.ChainMetadata{ + MCMAddress: mcmAddress, + AdditionalFields: []byte(`not-json`), + } + _, _, cerr := TimelockConverter{}.ConvertBatchToChainOperations(context.Background(), badMetadata, batchOp(), + timelockAddress, mcmAddress, types.NewDuration(time.Second), types.TimelockActionBypass, common.Hash{}, + common.HexToHash("0x01")) + require.Error(t, cerr) + require.ErrorContains(t, cerr, "unable to unmarshal solana-specific additional fields from chain metadata") + }) +} + +func TestApplyExecutePayerSignerOverride(t *testing.T) { + t.Parallel() + + a := solana.NewWallet().PublicKey() + b := solana.NewWallet().PublicKey() + accounts := []*solana.AccountMeta{ + {PublicKey: a, IsWritable: true}, + {PublicKey: b, IsWritable: true}, + } + + applyExecutePayerSignerOverride(accounts, b) + + require.False(t, accounts[0].IsSigner, "non-payer account must be untouched") + require.True(t, accounts[1].IsSigner, "payer account must be marked signer") + + t.Run("no match leaves accounts unchanged", func(t *testing.T) { + t.Parallel() + other := solana.NewWallet().PublicKey() + accts := []*solana.AccountMeta{{PublicKey: a, IsWritable: true}} + applyExecutePayerSignerOverride(accts, other) + require.False(t, accts[0].IsSigner) + }) + + t.Run("empty accounts is a no-op", func(t *testing.T) { + t.Parallel() + require.NotPanics(t, func() { + applyExecutePayerSignerOverride(nil, a) + applyExecutePayerSignerOverride([]*solana.AccountMeta{}, a) + }) + }) + + t.Run("skips nil entries without panicking", func(t *testing.T) { + t.Parallel() + accts := []*solana.AccountMeta{nil, {PublicKey: b, IsWritable: true}} + require.NotPanics(t, func() { + applyExecutePayerSignerOverride(accts, b) + }) + require.True(t, accts[1].IsSigner) + }) +} + +func TestGetAccountsFromBatchOperation(t *testing.T) { + t.Parallel() + + pk := solana.NewWallet().PublicKey() + programID := "11111111111111111111111111111111" + + t.Run("merges duplicate accounts as writable", func(t *testing.T) { + t.Parallel() + batch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{ + { + To: programID, + Data: []byte{1}, + AdditionalFields: toJSON(t, AdditionalFields{Accounts: []*solana.AccountMeta{ + {PublicKey: pk, IsWritable: false}, + }}), + }, + { + To: programID, + Data: []byte{2}, + AdditionalFields: toJSON(t, AdditionalFields{Accounts: []*solana.AccountMeta{ + {PublicKey: pk, IsWritable: true}, + }}), + }, + }, + } + accounts, err := getAccountsFromBatchOperation(batch) + require.NoError(t, err) + var found bool + for _, acc := range accounts { + if acc.PublicKey.Equals(pk) { + found = true + require.True(t, acc.IsWritable, "duplicate pubkey must OR IsWritable") + require.False(t, acc.IsSigner) + } + } + require.True(t, found) + }) + + t.Run("returns error for invalid additional fields json", func(t *testing.T) { + t.Parallel() + batch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: programID, + Data: []byte{1}, + AdditionalFields: []byte(`{not-json`), + }}, + } + _, err := getAccountsFromBatchOperation(batch) + require.Error(t, err) + require.ErrorContains(t, err, "unable to unmarshal additional fields") + }) +} + +func TestBypassRemainingAccounts(t *testing.T) { + t.Parallel() + + payer := solana.NewWallet().PublicKey() + baseMeta := AdditionalFieldsMetadata{ + ProposerRoleAccessController: solana.NewWallet().PublicKey(), + CancellerRoleAccessController: solana.NewWallet().PublicKey(), + BypasserRoleAccessController: solana.NewWallet().PublicKey(), + } + + validBatch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: "11111111111111111111111111111111", + Data: []byte{1}, + AdditionalFields: toJSON(t, AdditionalFields{Accounts: []*solana.AccountMeta{ + {PublicKey: payer, IsWritable: true}, + }}), + }}, + } + + t.Run("marks execute payer as signer", func(t *testing.T) { + t.Parallel() + accounts, err := bypassRemainingAccounts(validBatch, baseMeta.WithExecutePayer(payer)) + require.NoError(t, err) + require.Len(t, accounts, 2) // program id + payer + found := false + for _, acc := range accounts { + if acc.PublicKey.Equals(payer) { + found = true + require.True(t, acc.IsSigner) + } + } + require.True(t, found) + }) + + t.Run("without execute payer keeps non-signer", func(t *testing.T) { + t.Parallel() + accounts, err := bypassRemainingAccounts(validBatch, baseMeta) + require.NoError(t, err) + for _, acc := range accounts { + if acc.PublicKey.Equals(payer) { + require.False(t, acc.IsSigner) + } + } + }) + + t.Run("returns error for invalid program id", func(t *testing.T) { + t.Parallel() + badBatch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: "not-a-valid-solana-program-id", + Data: []byte{1}, + AdditionalFields: toJSON(t, AdditionalFields{}), + }}, + } + _, err := bypassRemainingAccounts(badBatch, baseMeta) + require.Error(t, err) + require.ErrorContains(t, err, "unable to parse program id") + }) + + t.Run("returns error for nil account in additional fields", func(t *testing.T) { + t.Parallel() + badBatch := types.BatchOperation{ + ChainSelector: chaintest.Chain4Selector, + Transactions: []types.Transaction{{ + To: "11111111111111111111111111111111", + Data: []byte{1}, + AdditionalFields: []byte(`{"accounts":[null]}`), + }}, + } + _, err := bypassRemainingAccounts(badBatch, baseMeta) + require.Error(t, err) + require.ErrorContains(t, err, "nil account in batch operation additional fields") + }) +} + func TestAppendIxDataChunkSize(t *testing.T) { tests := []struct { name string @@ -612,7 +959,7 @@ func TestOperationID(t *testing.T) { action: types.TimelockActionSchedule, predecessor: common.HexToHash("0x0123"), salt: common.HexToHash("0xabcd"), - wantErr: "unable to convert batch operation to solana instructions: unable to unmarshal Solana additional fields: invalid character", + wantErr: "unable to convert batch operation to solana instructions: unable to unmarshal additional fields: invalid character", }, } for _, tt := range tests { diff --git a/sdk/solana/timelock_executor_test.go b/sdk/solana/timelock_executor_test.go index 47af60a0..5b977351 100644 --- a/sdk/solana/timelock_executor_test.go +++ b/sdk/solana/timelock_executor_test.go @@ -114,9 +114,8 @@ func TestTimelockExecutor_Execute(t *testing.T) { //nolint:paralleltest }, setup: func(t *testing.T, e *TimelockExecutor, m *mocks.JSONRPCClient) { t.Helper() }, assertion: assertErrorEquals("unable to get InstructionData from batch operation: " + - "unable to unmarshal Solana additional fields: " + - "invalid character 'i' looking for beginning of value\n" + - "invalid JSON"), + "unable to unmarshal additional fields: " + + "invalid character 'i' looking for beginning of value"), }, { name: "error: invalid To program field", diff --git a/sdk/solana/transaction.go b/sdk/solana/transaction.go index 26fea03f..edb52780 100644 --- a/sdk/solana/transaction.go +++ b/sdk/solana/transaction.go @@ -2,6 +2,7 @@ package solana import ( "encoding/json" + "errors" "fmt" "math/big" @@ -13,14 +14,12 @@ import ( const rbacTimelockContractType = "RBACTimelock" +var errNilAccountInAdditionalFields = errors.New("nil account in batch operation additional fields") + func ValidateAdditionalFields(additionalFields json.RawMessage) error { - fields := AdditionalFields{ - Value: big.NewInt(0), - } - if len(additionalFields) != 0 { - if err := json.Unmarshal(additionalFields, &fields); err != nil { - return fmt.Errorf("failed to unmarshal solana additional fields: %w", err) - } + fields, err := ParseAdditionalFields(additionalFields) + if err != nil { + return err } return fields.Validate() @@ -31,6 +30,24 @@ type AdditionalFields struct { Value *big.Int `json:"value" validate:"omitempty"` } +// ParseAdditionalFields unmarshals raw JSON into AdditionalFields and validates +// that no account entry is nil. Returns a zero-value AdditionalFields when raw is empty. +func ParseAdditionalFields(raw json.RawMessage) (AdditionalFields, error) { + var fields AdditionalFields + if len(raw) > 0 { + if err := json.Unmarshal(raw, &fields); err != nil { + return AdditionalFields{}, fmt.Errorf("unable to unmarshal additional fields: %w", err) + } + } + for _, account := range fields.Accounts { + if account == nil { + return AdditionalFields{}, errNilAccountInAdditionalFields + } + } + + return fields, nil +} + // Validate ensures the solana-specific fields are correct func (f AdditionalFields) Validate() error { return validator.New().Struct(f) diff --git a/sdk/solana/transaction_test.go b/sdk/solana/transaction_test.go index 0ccd2d39..ad529b63 100644 --- a/sdk/solana/transaction_test.go +++ b/sdk/solana/transaction_test.go @@ -105,7 +105,7 @@ func TestValidateAdditionalFields(t *testing.T) { name: "malformed json", input: []byte(`invalid json`), wantErr: true, - errContains: "failed to unmarshal", + errContains: "unable to unmarshal additional fields", }, { name: "empty input",