Skip to content

Commit

Permalink
deployment/ccip: fix buggy test assertions (#15333)
Browse files Browse the repository at this point in the history
Tests have been underspecifying sequence numbers; we need to specify
both source and dest for a sequence number to be fully identifiable.
Fixing the helpers to force this by adding a SourceDestPair type that is
used as the key to all maps.
  • Loading branch information
makramkd authored Nov 20, 2024
1 parent 47da13e commit 5c7e945
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 26 deletions.
7 changes: 5 additions & 2 deletions deployment/ccip/changeset/active_candidate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestActiveCandidate(t *testing.T) {
// Need to keep track of the block number for each chain so that event subscription can be done from that block.
startBlocks := make(map[uint64]*uint64)
// Send a message from each chain to every other chain.
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)
for src := range e.Chains {
for dest, destChain := range e.Chains {
if src == dest {
Expand All @@ -53,7 +53,10 @@ func TestActiveCandidate(t *testing.T) {
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: nil,
})
expectedSeqNum[dest] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: src,
DestChainSelector: dest,
}] = msgSentEvent.SequenceNumber
}
}

Expand Down
7 changes: 5 additions & 2 deletions deployment/ccip/changeset/initial_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestInitialDeploy(t *testing.T) {
// Need to keep track of the block number for each chain so that event subscription can be done from that block.
startBlocks := make(map[uint64]*uint64)
// Send a message from each chain to every other chain.
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)

for src := range e.Chains {
for dest, destChain := range e.Chains {
Expand All @@ -102,7 +102,10 @@ func TestInitialDeploy(t *testing.T) {
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: nil,
})
expectedSeqNum[dest] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: src,
DestChainSelector: dest,
}] = msgSentEvent.SequenceNumber
}
}

Expand Down
35 changes: 26 additions & 9 deletions deployment/ccip/test_assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,24 @@ func ConfirmTokenPriceUpdated(
return nil
}

// SourceDestPair is represents a pair of source and destination chain selectors.
// Use this as a key in maps that need to identify sequence numbers, nonces, or
// other things that require identification.
type SourceDestPair struct {
SourceChainSelector uint64
DestChainSelector uint64
}

// ConfirmCommitForAllWithExpectedSeqNums waits for all chains in the environment to commit the given expectedSeqNums.
// expectedSeqNums is a map of destinationchain selector to expected sequence number
// expectedSeqNums is a map that maps a (source, dest) selector pair to the expected sequence number
// to confirm the commit for.
// startBlocks is a map of destination chain selector to start block number to start watching from.
// If startBlocks is nil, it will start watching from the latest block.
func ConfirmCommitForAllWithExpectedSeqNums(
t *testing.T,
e deployment.Environment,
state CCIPOnChainState,
expectedSeqNums map[uint64]uint64,
expectedSeqNums map[SourceDestPair]uint64,
startBlocks map[uint64]*uint64,
) {
var wg errgroup.Group
Expand All @@ -172,7 +181,11 @@ func ConfirmCommitForAllWithExpectedSeqNums(
startBlock = startBlocks[dstChain.Selector]
}

if expectedSeqNums[dstChain.Selector] == 0 {
expectedSeqNum, ok := expectedSeqNums[SourceDestPair{
SourceChainSelector: srcChain.Selector,
DestChainSelector: dstChain.Selector,
}]
if !ok || expectedSeqNum == 0 {
return nil
}

Expand All @@ -183,8 +196,8 @@ func ConfirmCommitForAllWithExpectedSeqNums(
state.Chains[dstChain.Selector].OffRamp,
startBlock,
ccipocr3.SeqNumRange{
ccipocr3.SeqNum(expectedSeqNums[dstChain.Selector]),
ccipocr3.SeqNum(expectedSeqNums[dstChain.Selector]),
ccipocr3.SeqNum(expectedSeqNum),
ccipocr3.SeqNum(expectedSeqNum),
})
})
}
Expand Down Expand Up @@ -307,7 +320,7 @@ func ConfirmExecWithSeqNrForAll(
t *testing.T,
e deployment.Environment,
state CCIPOnChainState,
expectedSeqNums map[uint64]uint64,
expectedSeqNums map[SourceDestPair]uint64,
startBlocks map[uint64]*uint64,
) (executionStates map[uint64]int) {
var (
Expand All @@ -328,7 +341,11 @@ func ConfirmExecWithSeqNrForAll(
startBlock = startBlocks[dstChain.Selector]
}

if expectedSeqNums[dstChain.Selector] == 0 {
expectedSeqNum, ok := expectedSeqNums[SourceDestPair{
SourceChainSelector: srcChain.Selector,
DestChainSelector: dstChain.Selector,
}]
if !ok || expectedSeqNum == 0 {
return nil
}

Expand All @@ -338,14 +355,14 @@ func ConfirmExecWithSeqNrForAll(
dstChain,
state.Chains[dstChain.Selector].OffRamp,
startBlock,
expectedSeqNums[dstChain.Selector],
expectedSeqNum,
)
if err != nil {
return err
}

mx.Lock()
executionStates[expectedSeqNums[dstChain.Selector]] = executionState
executionStates[expectedSeqNum] = executionState
mx.Unlock()

return nil
Expand Down
7 changes: 5 additions & 2 deletions integration-tests/smoke/ccip_messaging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,11 @@ func runMessagingTestCase(
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: extraArgs,
})
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum[tc.destChain] = msgSentEvent.SequenceNumber
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: tc.sourceChain,
DestChainSelector: tc.destChain,
}] = msgSentEvent.SequenceNumber
out.msgSentEvent = msgSentEvent

// hack
Expand Down
7 changes: 5 additions & 2 deletions integration-tests/smoke/ccip_rmn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func runRmnTestCase(t *testing.T, tc rmnTestCase) {

// Need to keep track of the block number for each chain so that event subscription can be done from that block.
startBlocks := make(map[uint64]*uint64)
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccipdeployment.SourceDestPair]uint64)
for _, msg := range tc.messagesToSend {
fromChain := chainSelectors[msg.fromChainIdx]
toChain := chainSelectors[msg.toChainIdx]
Expand All @@ -354,7 +354,10 @@ func runRmnTestCase(t *testing.T, tc rmnTestCase) {
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: nil,
})
expectedSeqNum[toChain] = msgSentEvent.SequenceNumber
expectedSeqNum[ccipdeployment.SourceDestPair{
SourceChainSelector: fromChain,
DestChainSelector: toChain,
}] = msgSentEvent.SequenceNumber
t.Logf("Sent message from chain %d to chain %d with seqNum %d", fromChain, toChain, msgSentEvent.SequenceNumber)
}

Expand Down
19 changes: 14 additions & 5 deletions integration-tests/smoke/ccip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestInitialDeployOnLocal(t *testing.T) {
// Need to keep track of the block number for each chain so that event subscription can be done from that block.
startBlocks := make(map[uint64]*uint64)
// Send a message from each chain to every other chain.
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)
for src := range e.Chains {
for dest, destChain := range e.Chains {
if src == dest {
Expand All @@ -44,7 +44,10 @@ func TestInitialDeployOnLocal(t *testing.T) {
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: nil,
})
expectedSeqNum[dest] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: src,
DestChainSelector: dest,
}] = msgSentEvent.SequenceNumber
}
}

Expand Down Expand Up @@ -90,7 +93,7 @@ func TestTokenTransfer(t *testing.T) {
// Need to keep track of the block number for each chain so that event subscription can be done from that block.
startBlocks := make(map[uint64]*uint64)
// Send a message from each chain to every other chain.
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)

twoCoins := new(big.Int).Mul(big.NewInt(1e18), big.NewInt(2))
tx, err := srcToken.Mint(
Expand Down Expand Up @@ -154,7 +157,10 @@ func TestTokenTransfer(t *testing.T) {
FeeToken: feeToken,
ExtraArgs: nil,
})
expectedSeqNum[dest] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: src,
DestChainSelector: dest,
}] = msgSentEvent.SequenceNumber
} else {
msgSentEvent := ccdeploy.TestSendRequest(t, e, state, src, dest, false, router.ClientEVM2AnyMessage{
Receiver: receiver,
Expand All @@ -163,7 +169,10 @@ func TestTokenTransfer(t *testing.T) {
FeeToken: feeToken,
ExtraArgs: nil,
})
expectedSeqNum[dest] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: src,
DestChainSelector: dest,
}] = msgSentEvent.SequenceNumber
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions integration-tests/smoke/ccip_usdc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func transferAndWaitForSuccess(
data []byte,
) {
startBlocks := make(map[uint64]*uint64)
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)

latesthdr, err := env.Chains[destChain].Client.HeaderByNumber(testcontext.Get(t), nil)
require.NoError(t, err)
Expand All @@ -227,7 +227,10 @@ func transferAndWaitForSuccess(
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: nil,
})
expectedSeqNum[destChain] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: sourceChain,
DestChainSelector: destChain,
}] = msgSentEvent.SequenceNumber

// Wait for all commit reports to land.
ccdeploy.ConfirmCommitForAllWithExpectedSeqNums(t, env, state, expectedSeqNum, startBlocks)
Expand Down
7 changes: 5 additions & 2 deletions integration-tests/smoke/fee_boosting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,18 @@ func runFeeboostTestCase(tc feeboostTestCase) {
require.NoError(tc.t, ccdeploy.AddLane(tc.deployedEnv.Env, tc.onchainState, tc.sourceChain, tc.destChain, tc.initialPrices))

startBlocks := make(map[uint64]*uint64)
expectedSeqNum := make(map[uint64]uint64)
expectedSeqNum := make(map[ccdeploy.SourceDestPair]uint64)
msgSentEvent := ccdeploy.TestSendRequest(tc.t, tc.deployedEnv.Env, tc.onchainState, tc.sourceChain, tc.destChain, false, router.ClientEVM2AnyMessage{
Receiver: common.LeftPadBytes(tc.onchainState.Chains[tc.destChain].Receiver.Address().Bytes(), 32),
Data: []byte("message that needs fee boosting"),
TokenAmounts: nil,
FeeToken: common.HexToAddress("0x0"),
ExtraArgs: nil,
})
expectedSeqNum[tc.destChain] = msgSentEvent.SequenceNumber
expectedSeqNum[ccdeploy.SourceDestPair{
SourceChainSelector: tc.sourceChain,
DestChainSelector: tc.destChain,
}] = msgSentEvent.SequenceNumber

// hack
time.Sleep(30 * time.Second)
Expand Down

0 comments on commit 5c7e945

Please sign in to comment.