Skip to content

Commit

Permalink
fix(solana): fix simulator side effect bug
Browse files Browse the repository at this point in the history
If we create a simulator with an executor, on construction of that simulator, the sendAndConfirm function is overwritten with a no-op version.

```
executor := mcmsSolana.NewExecutor(s.SolanaClient, auth, encoder)
simulator := mcmsSolana.NewSimulator(executor)
simulator.SimulateSetRoot(....)

executor.SetRoot(....) ????? This will not perform any transaction
```

This commit ensure a copy of executor is modified.

Side note: i combined the e2e simulator test with the e2e executor test so we can simulate and perform the actual operation, not to mention we saved a bunch of setup.

JIRA: https://smartcontract-it.atlassian.net/browse/DPA-1417
  • Loading branch information
graham-chainlink committed Jan 31, 2025
1 parent f3545dd commit 185145e
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 119 deletions.
5 changes: 5 additions & 0 deletions .changeset/real-kangaroos-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@smartcontractkit/mcms": patch
---

fix(solana): fix simulator side effect bug
32 changes: 31 additions & 1 deletion e2e/tests/solana/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package solanae2e
import (
"context"
"math/big"
"slices"
"time"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -88,7 +89,8 @@ func (s *SolanaTestSuite) Test_Solana_Execute() {
encoders, err := proposal.GetEncoders()
s.Require().NoError(err)
encoder := encoders[s.ChainSelector].(*mcmsSolana.Encoder)
executors := map[types.ChainSelector]sdk.Executor{s.ChainSelector: mcmsSolana.NewExecutor(encoder, s.SolanaClient, auth)}
executor := mcmsSolana.NewExecutor(encoder, s.SolanaClient, auth)
executors := map[types.ChainSelector]sdk.Executor{s.ChainSelector: executor}
inspectors := map[types.ChainSelector]sdk.Inspector{s.ChainSelector: mcmsSolana.NewInspector(s.SolanaClient)}

// sign proposal
Expand All @@ -103,6 +105,34 @@ func (s *SolanaTestSuite) Test_Solana_Execute() {
_, err = configurer.SetConfig(ctx, mcmID, &mcmConfig, true)
s.Require().NoError(err)

// simulate SetRoot
metadata := proposal.ChainMetadata[s.ChainSelector]
metadataHash, err := encoder.HashMetadata(metadata)
s.Require().NoError(err)

tree, err := proposal.MerkleTree()
s.Require().NoError(err)

proof, err := tree.GetProof(metadataHash)
s.Require().NoError(err)

hash, err := proposal.SigningHash()
s.Require().NoError(err)

// Sort signatures by recovered address
sortedSignatures := slices.Clone(proposal.Signatures) // Clone so we don't modify the original
slices.SortFunc(sortedSignatures, func(a, b types.Signature) int {
recoveredSignerA, _ := a.Recover(hash)
recoveredSignerB, _ := b.Recover(hash)

return recoveredSignerA.Cmp(recoveredSignerB)
})

simulator := mcmsSolana.NewSimulator(executor)
err = simulator.SimulateSetRoot(ctx, "", metadata,
proof, [32]byte(tree.Root.Bytes()), proposal.ValidUntil, sortedSignatures)
s.Require().NoError(err)

// call SetRoot
executable, err := mcms.NewExecutable(proposal, executors)
s.Require().NoError(err)
Expand Down
111 changes: 0 additions & 111 deletions e2e/tests/solana/simulator.go

This file was deleted.

4 changes: 2 additions & 2 deletions sdk/solana/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ func NewExecutor(encoder *Encoder, client *rpc.Client, auth solana.PrivateKey) *
}
}

func (e *Executor) withSendAndConfirmFn(fn SendAndConfirmFn) *Executor {
func (e Executor) withSendAndConfirmFn(fn SendAndConfirmFn) *Executor {
e.sendAndConfirm = fn
return e
return &e
}

// ExecuteOperation executes an operation on the MCMS program on the Solana chain
Expand Down
24 changes: 21 additions & 3 deletions sdk/solana/simulator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,28 @@ func TestSimulator_SimulateSetRoot(t *testing.T) {
}

tests := []struct {
name string
setupMocks func(t *testing.T, client *mocks.JSONRPCClient)
expectedError string
name string
setupMocks func(t *testing.T, client *mocks.JSONRPCClient)
executeSetRoot bool
expectedError string
}{
{
name: "success",
setupMocks: func(t *testing.T, client *mocks.JSONRPCClient) {
t.Helper()
mockSolanaSimulateTransaction(t, client, 50, nil, nil)

// for real execution
mockSolanaTransaction(t, client, 50, 60,
"AxzwxQ2DLR4zEFxEPGaafR4z3MY4CP1CAdSs1ZZhArtgS3G4F9oYSy3Nx1HyA1Macb4bYEi4jU6F1CL4SRrZz1v", nil, nil)
mockSolanaTransaction(t, client, 51, 61,
"3DqeyZzb7PJmQ31M1qdXfP7iACr5AiEXcKeLUNmVvDoYM23JJK5ZvermxsDy8eiQKzpagc69MKRtrpzK7tRcLGgr", nil, nil)
mockSolanaTransaction(t, client, 52, 62,
"GHR9z23oUJnS2aV5HZ9zEpUpEe4qoBLFMzuBjNA3xJcQuc6JjDmuUq2VVmxqwPeFzfs8V7nfjqc1wRviEb82bRu", nil, nil)
mockSolanaTransaction(t, client, 53, 63,
"oaV9FKKPDVneUANQ9hJqEuhgwfUgbxucUC4TmzpgGJhuSxBueapWc9HJ4cJQMqT2PPQX6rhTbKnXkebsaravnLo", nil, nil)
},
executeSetRoot: true,
},
{
name: "failure: GetLastBlockhash error",
Expand Down Expand Up @@ -75,6 +87,12 @@ func TestSimulator_SimulateSetRoot(t *testing.T) {
} else {
require.NoError(t, err)
}

if tt.executeSetRoot {
result, err := executor.SetRoot(ctx, metadata, proof, root, validUntil, signatures)
require.NoError(t, err)
require.NotNil(t, result)
}
})
}
}
4 changes: 2 additions & 2 deletions sdk/solana/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func mockSolanaSimulateTransaction(
}}

return mockBlockHashRPCError
})
}).Once()
if mockBlockHashRPCError != nil {
return
}
Expand All @@ -195,7 +195,7 @@ func mockSolanaSimulateTransaction(
}

return nil
})
}).Once()
}

var sendTransactionParams = func(t *testing.T) any {
Expand Down

0 comments on commit 185145e

Please sign in to comment.