-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add ExecutePayer field in chain metadata for setting solana execute accounts [CLD-3046] #814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
0ea5199
feat: add WithExecutePayers ctx function for setting solana accounts …
ecPablo 7d3ab9f
fix: linting errors
ecPablo 7d2f102
fix: linting errors
ecPablo d9103a6
fix: linting errors
ecPablo 8e3e1ab
fix: linting errors
ecPablo 8d11e70
Potential fix for pull request finding
ecPablo 0f00101
Potential fix for pull request finding
ecPablo af05ac5
fix: linting errors
ecPablo 316e5c0
feat: update approach to get execute payers from chain metadata inste…
ecPablo b4223f9
feat: update docs
ecPablo f1d61e2
fixc: increase unit test coverage
ecPablo 505017e
Potential fix for pull request finding
ecPablo 6a93d97
Potential fix for pull request finding
ecPablo 4ab690e
fix: copilot comments
ecPablo c26f14e
fix: copilot comments
ecPablo 0391a1a
Potential fix for pull request finding
ecPablo ce50685
Potential fix for pull request finding
ecPablo 6b1f4f5
fix: address review comments
ecPablo d30e0ef
fix: address review comments
ecPablo 0060afb
Potential fix for pull request finding
ecPablo c8c1d88
Potential fix for pull request finding
ecPablo 031122f
fix: address review comments
ecPablo 31f55cd
fix: unit tests
ecPablo 660b768
Merge branch 'main' into ecpablo/add-execute-payers-solana
ecPablo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"). | ||
|
Copilot marked this conversation as resolved.
|
||
| 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) | ||
| } | ||
|
Copilot marked this conversation as resolved.
|
||
|
|
||
| // 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 | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.