Conversation
patricios-space
force-pushed
the
feat/block-low-gaslimit-msg
branch
2 times, most recently
from
97e6311 to
56bc0b0
Compare
There was a problem hiding this comment.
Pull request overview
Adds contract-side enforcement to block CCIP executions when the message gasLimit is below a minimum threshold, and updates bindings/docs/tests to reflect the new behavior.
Changes:
- Introduces a
MIN_GASLIMITthreshold in the OffRamp contract and bounces the ReceiveExecutor flow whengasLimitis too low. - Extends OffRamp error/binding enums and stringers to include a new
GasLimitTooLowcode. - Updates Mermaid flow documentation and adds a unit test covering the low-gasLimit blocked execution path.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/ccip/bindings/offramp/offramp.go | Extends Go exit-code range and enum with ErrorGasLimitTooLow. |
| pkg/ccip/bindings/offramp/exitcode_string.go | Regenerates ExitCode stringer data to include the new exit code. |
| docs/contracts/overview/ccip/offramp/arbitrary-msg.md | Documents the new minimum gasLimit verification / bounce path in the sequence diagram. |
| contracts/wrappers/ccip/ReceiveExecutor.ts | Adds exported opcode constants used by tests to identify bounced messages. |
| contracts/wrappers/ccip/OffRamp.ts | Adds ExecutionState enum and extends OffRampError with GasLimitTooLow. |
| contracts/tests/ccip/OffRamp.spec.ts | Adds a unit test asserting the low-gasLimit execution is bounced and state transitions occur. |
| contracts/contracts/ccip/offramp/errors.tolk | Adds GasLimitTooLow to the contract error enum. |
| contracts/contracts/ccip/offramp/contract.tolk | Implements the MIN_GASLIMIT check and a helper that sends ReceiveExecutor_Bounced. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fun blockMsg(st: Storage, execId: uint192, receiver: address) { | ||
| val receiveBounced = createMessage({ | ||
| bounce: true, | ||
| value: 0, | ||
| dest: getReceiverExecutorDeployAddress(st, execId), | ||
| body: ReceiveExecutor_Bounced{ | ||
| receiver: receiver | ||
| } | ||
| }); | ||
| receiveBounced.send(SEND_MODE_CARRY_ALL_REMAINING_MESSAGE_VALUE); | ||
| } |
There was a problem hiding this comment.
blockMsg duplicates the existing “send ReceiveExecutor_Bounced” logic already present in this contract (e.g., Router_RouteMessage bounce handling). Consider factoring this into a single helper (or reusing an existing one) to avoid having to update multiple places if the bounced-notification flow changes (body fields, send mode, bounce mode, etc.).
There was a problem hiding this comment.
This is true but also we want to reduce the number of lines changed, right?
patricios-space
force-pushed
the
feat/block-low-gaslimit-msg
branch
from
56bc0b0 to
a036f28
Compare
patricios-space
force-pushed
the
feat/block-low-gaslimit-msg
branch
from
a036f28 to
addc67d
Compare
| routeMessage.send(SEND_MODE_REGULAR); | ||
| } | ||
|
|
||
| fun blockMsg(st: Storage, execId: uint192, receiver: address) { |
There was a problem hiding this comment.
Why not reuse the onBouncedRouterRouteMessage which already does this + emits an event?
Makes sense as we're basically producing a synthetic bounce, before hitting the receiver.
The only additional thing would be differentiating the errors/bounces (native vs. synthetic) - this could be a different topic or OffRamp_RouteMessageBounced struct could be extended with info (bool: synthetic, int: exitCode)
There was a problem hiding this comment.
onBouncedRouterRouteMessage is called when Router bounces Router_RouteMessage. I think onBouncedCCIPReceive would be more appropiate, as it is the one called on the receiver bouncing. Either way, I was reluctant to use any of these as they both load storage again, which is inneficient. I could refactor them to pass storage as a parameter, but I was aiming at introducint the fewest number of changes nesessary.
NONEVM-4455