-
Notifications
You must be signed in to change notification settings - Fork 4
Unit Tests #430
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
Unit Tests #430
Conversation
* ref: sorting imports * fix: typo * ref: extract setup function * ref: move constants to setup * ref: split apply ramp updates * wip: Generic router setup * fix: missing bracket * fix: rm unsued imports * fix: export selectors * test: opcodes * ref: join outopcodes * ref: join opcodes * test: out opcodes * fix: update opcodes to crc32 * ref: split main test * lint: sort imports * ref: move e2e to separate suite * ref: rename send e2e test
* fix: wrong error * doc: update comment * lint: change error name
* wip: marking * test: isChainSupported * test: expectedNextSequenceNumber * test: getDestChainConfig * test: getDestChainSelectors * test: getExecutorCodeHash * test: getFacilityId & getErrorCode * ref: rename getter method * Update contracts/wrappers/ccip/OnRamp.ts Co-authored-by: Kristijan Rebernisak <kristijan.rebernisak@gmail.com> * fmt: lint last change --------- Co-authored-by: Kristijan Rebernisak <kristijan.rebernisak@gmail.com>
* test: out opcodes * ref: join opcodes * fix: update onramp opcodes to crc32 * fix: rm duplicated tests
…ssing dest chain config (#424) * feat: return message rejected instead of * fmt: lint * ref: rm unnecessary constant * ref: extracting router * fix: opcode struct change * fix: opcode struct change
…lowlist (#425) * feat: return message rejected instead of throwing on allowlist * fix: redudant valueOf
* test: getPendingOwner * fix: passing with null pending owner
* ref: rename send executor tests * ref: extract setup function * ref: sort imports * test: opcodes * test: facility ID and error code * test: use deployable * test: execute * test: execute unauthorized * test: double exec * test: messageValidated * test: messageValidationFailed * test: bounced messages * ref: rm unused imports * fix: potentiall re-entrancy vulnerability
* feat: missing errors * test: send after uncurse * test: ccipSend * test: fwd message sent and rejected * test: getFee * test: add rejection case for getValidatedFee on disabled dest chain * test: messageValidated and messageValidationFailed * fix: opcode struct * fix: circular dependency constant initialization * test: getDestChainSelectors * test: getVerifyNotCursed * ref: fix rmn bindings * test: verifyNotCursed * fix: rm inexistant opcodes * fix: update test coverage * fix: test coverage * test: ccipReceive * test: ccipReceiveConfirm * fix: missing ownable test coverage * test: error code and facility ID * fix: test coverage for applyRampUpdates * fix: test coverage for applyRampUpdates * test: sendApplyRampUpdatesSetRamps offRampRemoves * fix: sourcChainSelector lookup should throw SourceChainNotEnabled * rmnRemote ownable (#437) * feat: rmn ownable TS bindings * ref: inject role in ownable wrapper * chore: rm unsued imports * fix: duplicated opcode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements unit tests and fixes various issues in the CCIP contracts, focusing on correcting error handling, updating opcodes to use CRC32 values, and consolidating duplicate structures.
Key Changes
- Updated message opcodes across Router, OnRamp, and SendExecutor to use CRC32 of their names instead of arbitrary values
- Fixed error handling in OnRamp to reject messages instead of throwing errors for missing configurations and allowlist violations
- Consolidated duplicate message structures in OnRamp Go bindings
- Added new error codes for address validation (EVM, 32-byte, SVM receiver addresses)
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/ccip/bindings/router/router.go | Updated Router opcodes to CRC32 values |
| pkg/ccip/bindings/onramp/onramp.go | Updated OnRamp opcodes and merged duplicate structs |
| pkg/ccip/bindings/feequoter/fee_quoter.go | Added new receiver address validation error codes |
| pkg/ccip/bindings/ccipsendexecutor/ccipsendexecutor.go | Added FeeQuoterBounce error code |
| contracts/wrappers/ccip/Router.ts | Updated opcodes and added RMN ownership support |
| contracts/wrappers/ccip/OnRamp.ts | Updated opcodes and added getter methods |
| contracts/wrappers/ccip/FeeQuoter.ts | Renamed error enum and removed unused error constant |
| contracts/wrappers/ccip/CCIPSendExecutor.ts | Added FeeQuoterBounce error and finalized state |
| contracts/tests/ccip/sendExecutor/SendExecutor.spec.ts | Added comprehensive unit tests for SendExecutor |
| contracts/tests/ccip/router/*.spec.ts | Split Router tests into focused test files |
| contracts/contracts/ccip/router/contract.tolk | Fixed error code for missing source chain config |
| contracts/contracts/ccip/onramp/contract.tolk | Changed to reject messages instead of throwing errors |
| contracts/contracts/ccip/fee_quoter/contract.tolk | Updated address validation to use correct error codes |
| contracts/contracts/ccip/ccipsend_executor/contract.tolk | Added finalized state and FeeQuoterBounce error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* feat: fail on commit with multiple roots * ref: remove unnecesary variable
vicentevieytes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the fixes look good
This reverts commit 5e2ce34.
Changes
Contract Changes (+91, -41)
FeeQuoter
InvalidSVMReceiverAddress: svm address validation should throwInvalidSVMReceiverAddressinstead ofInvalidSuiReceiverAddress. See new error here.InvalidEVMAddressandInvalid32ByteAddress) to FeeQuoter errors. See removal and uses. See also new errorsvalidate32ByteAddress():validate32ByteAddress()should throwInvalidEVMReceiverAddressOnRamp
OnRamp_Send,OnRamp_SetDynamicConfig,OnRamp_UpdateDestChainConfigs)getCCIPSendExecutorCodeHashforsendExecutorCodeHashto be consistentRouter
Router_ApplyRampUpdates,Router_CCIPSend,Router_CCIPReceiveConfirm,Router_RMNRemoteCurse,Router_RMNRemoteUncurse,Router_RMNRemoteVerifyNotCursed)st.offRamps.mustGet()should throwRouter_Error.SourceChainNotEnabledinstead ofDestChainNotEnabledrmn_owner()andrmn_pendingOwner()getters.SendExecutor
FeeQuoterBounceerror onFeeQuoter_GetValidatedFeebounce. New error hereOffRamp
Go Bindings (+21, -28)
FeeQuoter
ErrorInvalidEVMReceiverAddress,ErrorInvalid32ByteReceiverAddressandErrorInvalidSVMReceiverAddressOnRamp
SetDynamicConfig->SetDynamicConfigMessageandUpdateDestChainConfigs->UpdateDestChainConfigsMessageUpdateDestChainConfigsMessage,SendandSetDynamicConfigMessageRouter
ApplyRampUpdates,CCIPSendandCCIPReceiveConfirmSendExecutor:
ErrorFeeQuoterBounceOffRamp: