Conversation
vicentevieytes
force-pushed
the
vv/fire-drills
branch
from
044723a to
8b3c970
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces firedrill contracts - a testing framework for simulating CCIP message flows on TON. The implementation includes three main contracts (OnRamp, OffRamp, and Entrypoint) that emit events and facilitate end-to-end testing of the CCIP protocol without full deployment complexity.
Changes:
- Added three new firedrill contracts (OnRamp, OffRamp, Entrypoint) with TypeScript wrappers and Tolk implementations
- Enhanced log handling to support new price update events (UsdPerTokenUpdated, UsdPerUnitGasUpdated)
- Fixed OffRamp buffer loading logic to use remainingBits instead of explicit length
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/wrappers/firedrill/FiredrillOnRamp.ts | TypeScript wrapper for OnRamp contract with message emission functionality |
| contracts/wrappers/firedrill/FiredrillOffRamp.ts | TypeScript wrapper for OffRamp contract with commit report handling |
| contracts/wrappers/firedrill/FiredrillEntrypoint.ts | Main entrypoint contract managing firedrill flow coordination |
| contracts/wrappers/firedrill.*.compile.ts | Compilation configuration files for the three firedrill contracts |
| contracts/wrappers/ccip/OffRamp.ts | Updated buffer loading to use remainingBits for cross-chain addresses |
| contracts/wrappers/ccip/Logs.ts | Added new log types and handlers for price update events |
| contracts/tests/firedrill/*.spec.ts | Comprehensive test suites for all firedrill contracts |
| contracts/tests/firedrill/Firedrill.Setup.ts | Test setup utilities and helper functions |
| contracts/tests/ccip/OffRamp.spec.ts | Added test verification for onRamp address after update |
| contracts/tests/Logs.ts | Added handlers for new price update log types |
| contracts/scripts/deployFiredrillComplete.ts | Deployment script for complete firedrill system |
| contracts/contracts/firedrill/*.tolk | Tolk contract implementations for firedrill system |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const modifiedMsg = { | ||
| sourceChainSelector: msg.sourceChainSelector, | ||
| config: { | ||
| router: msg.config.router.toString(), | ||
| isEnabled: msg.config.isEnabled, | ||
| minSeqNr: msg.config.minSeqNr, | ||
| isRMNVerificationDisabled: msg.config.isRMNVerificationDisabled, | ||
| onRamp: msg.config.onRamp, | ||
| } | ||
| } | ||
|
|
||
| const modifiedMatch = { | ||
| sourceChainSelector: match.sourceChainSelector, | ||
| config: { | ||
| router: match.config.router.toString(), | ||
| isEnabled: match.config.isEnabled, | ||
| minSeqNr: match.config.minSeqNr, | ||
| isRMNVerificationDisabled: match.config.isRMNVerificationDisabled, | ||
| onRamp: match.config.onRamp, | ||
| } | ||
| } |
There was a problem hiding this comment.
The code attempts to call .toString() on match.config.router but doesn't verify that match.config.router exists or is an Address before calling the method. If the router property is undefined in the partial match object, this will throw a runtime error. Add a null/undefined check or use optional chaining: router: match.config?.router?.toString()
contracts/tests/Logs.ts
Outdated
| matchesObject(msg, match) | ||
| return true | ||
| }) | ||
|
|
There was a problem hiding this comment.
The testLogUsdPerUnitGasUpdated function has an empty line before the closing brace and after the return statement (line 542). This is inconsistent with the style of other similar functions in this file. Remove the empty line on line 542.
| struct FiredrillEntrypoint_Storage { | ||
| id: uint32; |
There was a problem hiding this comment.
I think we don't need the firedrill_* prefix on these contracts. They are already inside the firedrill directory.
There was a problem hiding this comment.
This is done throughout the firedrill contracts for the rest of the chains, I think it makes sense to keep it this way
| } | ||
|
|
||
| enum FiredrillEntrypoint_Error { | ||
| Unauthorized = 3000, |
| const entrypointCode = await compile('firedrill.entrypoint'); | ||
| const onRampCode = await compile('firedrill.onramp'); | ||
| const offRampCode = await compile('firedrill.offramp'); | ||
| console.log('✅ Contracts compiled'); |
There was a problem hiding this comment.
You can use already built bytecode with loadContractCode
There was a problem hiding this comment.
These compile really quickly and we won't be redeploying them often, I prefer to have them compile, that way we are sure that we did not deploy any outdated bytecode because we forgot to build
|
@bukata-sa could you help review from a firedrill setup standpoint once you get a chance please 🙏 |
3 participants
No description provided.