Init ccipReader e2e tests #1203
Conversation
dimkouv
requested review from
a team,
makramkd,
elatoskinas and
RayXpub
as code owners
LCOV of commit
|
go.sum
Outdated
| github.com/smartcontractkit/chainlink-ccip v0.0.0-20240708100035-6d7cad39cc74 h1:dsRsqoCRF+SrIvARgJXs6RQcJ6oGfh3XigIfrrEWO2E= | ||
| github.com/smartcontractkit/chainlink-ccip v0.0.0-20240708100035-6d7cad39cc74/go.mod h1:gyODeD1uMobe5VWRwVRiEXzL9wUzFTI80VvyCNLqWcw= | ||
| github.com/smartcontractkit/chainlink-ccip v0.0.0-20240716114142-65c4a791cd3d h1:k4+p4XTsws++V369bWzfnEut3A5mxKxqbMBZKfIF/2I= |
| }, | ||
| }) | ||
| assert.NoError(t, err) | ||
| err = cr.Start(ctx) |
There was a problem hiding this comment.
One liner for this is good. Also using require makes it stop instead of continuing with the rest of the tests afterwards and failing all of them making the logs pretty noisy.
| err = cr.Start(ctx) | |
| require.NoError(t, cr.Start(ctx)) |
There was a problem hiding this comment.
Same applies for the rest of the test
| contractAddr common.Address | ||
| contract *ccip_reader_tester.CCIPReaderTester |
There was a problem hiding this comment.
Nit,
Remove address as you already can get it from the contract like contract.Address()
rename contract for readability
| contractAddr common.Address | |
| contract *ccip_reader_tester.CCIPReaderTester | |
| ccipTester *ccip_reader_tester.CCIPReaderTester |
| bytes onRamp; | ||
| } | ||
|
|
||
| struct EVM2AnyRampMessage { |
There was a problem hiding this comment.
suggestion: we can use the structs directly, they are already defined:
import {Internal} from "../../libraries/Internal.sol";Would it cause issues off-chain if we use the full EVM2AnyRampMessage struct? (for the header - the struct is exactly as you've defined it in L16-22 so we can definitely de-dupe it)
| pragma solidity 0.8.24; | ||
|
|
||
| contract CCIPReaderTester { | ||
| struct SourceChainConfig { |
There was a problem hiding this comment.
suggestion: we can use the struct directly:
import {EVM2EVMMultiOffRamp} from "../../offRamp/EVM2EVMMultiOffRamp.sol";Usage:
EVM2EVMMultiOffRamp.SourceChainConfig| s_sourceChainConfigs[sourceChainSelector] = sourceChainConfig; | ||
| } | ||
|
|
||
| event CCIPSendRequested(uint64 indexed destChainSelector, EVM2AnyRampMessage message); |
There was a problem hiding this comment.
suggestion: I think you might also be able to use the event directly without re-defining it: EVM2EVMMultiOffRamp.CCIPSendRequested
Though this might be limited to test files
| evmtypes "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types" | ||
| ) | ||
|
|
||
| func TestCCIPReader_CommitReportsGTETimestamp(t *testing.T) {} |
There was a problem hiding this comment.
q: tests are empty - missing TODOs?
| for sourceChain, seqNum := range onChainSeqNums { | ||
| _, err := contract.SetSourceChainConfig(auth, uint64(sourceChain), ccip_reader_tester.CCIPReaderTesterSourceChainConfig{ | ||
| IsEnabled: true, | ||
| MinSeqNr: uint64(seqNum), |
There was a problem hiding this comment.
q: minSeqNr is actually not settable on the OffRamp. Does it make sense to set it in the tests?
Init test helper contract and add e2e test for ccipReader NextSeqNum method.