Fix receipt marking gas cost - #1465
Open
geoff-vball wants to merge 5 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
geoff-vball
marked this pull request as ready for review
August 27, 2026 17:14
geoff-vball
requested review from
SavalaBee,
batconjurer,
harri-ava,
iansuvak,
mdelle1 and
ylg-avalabs
August 27, 2026 17:14
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a relayer griefing/cost-amplification vector by (1) correctly accounting for receipt-marking gas in Teleporter deliveries and (2) introducing a “non-retryable” error classification so deterministic delivery failures don’t burn the full retry budget or crash-loop the relayer.
Changes:
- Add
messages.ErrNonRetryable+ specific non-retryable causes, and teach the relayer to stop retrying / checkpoint past deterministic failures while recording a dedicated “abandoned” metric. - Update Teleporter
ShouldSendMessage/SendMessageto (a) reject receipt-heavy deliveries that can’t fit in a block and (b) classify out-of-gas reverts as non-retryable. - Re-derive and raise
MarkMessageReceiptGasCost, add a Solidity benchmark test to guard it, and fix batch relay required gas limit semantics in contract flow tests.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| relayer/application_relayer.go | Stops retries on ErrNonRetryable and skips non-retryable messages in ProcessHeight so heights can still checkpoint. |
| relayer/application_relayer_test.go | Adds unit tests covering non-retryable retry suppression and height processing behavior. |
| relayer/application_relayer_metrics.go | Introduces abandoned_relay_message_count and exposes it via the relayer metrics adapter. |
| messages/teleporter/message_handler.go | Adds receipt-aware block gas checks and emits non-retryable errors for block-limit and out-of-gas deterministic failures. |
| messages/teleporter/message_handler_test.go | Extends tests for receipt-heavy messages and non-retryable out-of-gas detection. |
| messages/mocks/mock_message_handler.go | Updates the generated metrics mock to include IncAbandonedRelayMessageCount. |
| messages/message_handler.go | Defines ErrNonRetryable, specific non-retryable causes, and a bounded NonRetryableReason mapping for metrics. |
| icm-contracts/utils/gas-utils/gas_utils.go | Updates MarkMessageReceiptGasCost and refines benchmark/documentation comments for the estimation formula. |
| icm-contracts/tests/flows/services/batch_relay.go | Fixes required gas limit to be per-message (not multiplied by batch size). |
| icm-contracts/avalanche/teleporter/tests/MarkReceiptGasBenchmarkTests.t.sol | Adds a benchmark test to validate/guard the per-receipt gas budget used by the relayer. |
Files not reviewed (1)
- messages/mocks/mock_message_handler.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
batconjurer
requested changes
Aug 28, 2026
| contract MarkReceiptGasBenchmarkTest is TeleporterMessengerTest { | ||
| // Sample sizes. Marginal cost comes from differences between them, so the fixed cost of the | ||
| // receive path cancels out. Three sizes rather than two so the result also shows whether the | ||
| // cost is linear in the receipt count - if it were superlinear, no per-receipt constant in |
Contributor
There was a problem hiding this comment.
nit: I would adjust the comment slightly to make clear that linear is the expected behavior.
| function _stageReceipts( | ||
| TeleporterMessageReceipt[] storage receipts, | ||
| uint256 count, | ||
| uint256 salt |
Contributor
There was a problem hiding this comment.
nit: To harden against accidental salt reuse, a global salt counter could be used and incremented within this function. Then the salt would not be exposed as a parameter.
| // A delivery that reverts having consumed its entire gas limit ran out of gas. That is | ||
| // deterministic, so the relayer must surface it as non-retryable rather than re-broadcasting and | ||
| // paying for the same reverted transaction again. | ||
| func TestSendMessageOutOfGasIsNonRetryable(t *testing.T) { |
Contributor
There was a problem hiding this comment.
can we get a boundary test on this? So if we reduce the number of receipts slightly is does get relayed?
Comment on lines
+123
to
+136
| function _stageReceipts(TeleporterMessageReceipt[] storage receipts, uint256 count) private { | ||
| for (uint256 i; i < count; ++i) { | ||
| uint256 nonce = _getNextMessageNonce(); | ||
| _sendTestMessageWithFee(DEFAULT_SOURCE_BLOCKCHAIN_ID, 1 ether); | ||
| receipts.push( | ||
| TeleporterMessageReceipt({ | ||
| receivedMessageNonce: nonce, | ||
| relayerRewardAddress: address( | ||
| uint160(uint256(keccak256(abi.encode(_nextRewardAddressSeed++)))) | ||
| ) | ||
| }) | ||
| ); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this should be merged
Closes https://claude.ai/security?project=scanproj_019LCMtLJaK3JHExaMcFeX7Z&finding=scan_018ZtbcWNnFGkytCFgch5FPX-3492944
How this works
Introduces a non-retryable error - deterministic message send failures that will never resolve. We let the message fail and checkpoint the block anyway to prevent potential griefing attacks
How this was tested
How is this documented