Repository navigation
docs: fix #40 and #43, and correct stale claims across the docs - #45
Merged
Merged
Conversation
The bar is checked in TransceiverBase._counterpartOn, so it applies to every message a transceiver sends to or accepts from a chain, not only the first: a bootstrap, a receiver report, and an inbound delivery. The bootstrap NatSpec, spec C19, and message-flow.md said otherwise. Adds the missing test: a report to a home graded below the bar is refused, and the account creation reverts with it. Fixes #40
R1, the spec's override table, and OutboundBase._sendMessage's NatSpec said a binding returns "the gateway's sendId". That holds for a binding over an ERC-7786 gateway (§9), but read for a native binding it means the provider's own id, which ProviderSendSpec forbids and no binding returns. They now say zero once the message is away, and keep the gateway case separate. Fixes #43
… guard order The config field is governorOwner and the docs say governor; NatSpec said msig. ReceiverBase said a binding configures its provider after the reentrancy guard, but bindings configure before __ReceiverBase_init, which initializes the guard. Comment-only: deployed bytecode of every src contract is unchanged.
- Terms: a route is a chain's ERC-7930 identifier; provider ids live in ProviderChainId. R5.1 and the checklist no longer describe routes as provider-width codec values. - P9 names each binding's actual quote (Wormhole: Core messageFee plus the Executor's quoteExecution, not the deprecated Relayer); Axelar dropped. - R2.3: only LayerZero, CCIP, and Hyperlane price per byte. - R3 table: native receivers funnel through _onMessageFrom. - R6.3: provider setup runs before the reentrancy guard is initialized. - R8.4 and §6: the shared deploy asserts the record; one source for N x N. - §8: C1 and C2 state what the shared tests assert; C27 cites tests that exist; fork tests are C11 and C29 to C31; test/vectors is not built. - §4.6 renumbered to 4.5; msig becomes governor.
- message-flow: a bare commit is 288 bytes, not 320; the inheritance diagram no longer puts ReceiverBase under OutboundBase; Unknown is accepted by the registry and passes a bar of Unknown. - encoding: test/vectors is planned, not built; quotations match the code they cite. - research: LzTransmitter calls __OAppSender_init; phase labels dropped; the OP Stack per-pair rationale notes its reversal (#41). - todo: #41 listed; every provider's default gas named. - README: only CrossProxyDeployer comes from Arachnid's factory; the N x N source is deploy/; msig becomes governor; OP Stack notes #41.
pegahcarter
added this pull request to stack #48
October 6, 2026 08:03
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Changes are documentation, source comments (bytecode-identical), and one additive test that I verified compiles against the real APIs and correctly exercises the provenance bar, with all spot-checked doc claims and cross-references confirmed accurate.
Review effort: Balanced
Findings: None
What changed in this PR
This PR corrects stale or inaccurate documentation across the repo and closes two documentation issues. It fixes #40 (the provenance bar gates every message a transceiver sends to or accepts from a chain — bootstrap, receiver report, and inbound delivery — not just the first bootstrap) and #43 (a native binding's _sendMessage returns zero, ERC-7786's "sent", never the provider's own message id). It also sweeps the docs for stale claims (route definition, P9 quote functions, the inheritance diagram, encoding byte counts, planned-vs-built test/vectors/, OP Stack #41 caveats) and renames "msig" to "governor" in docs and source NatSpec to match the config's governorOwner. The only non-comment code change is one additive test.
Changes:
- Corrects #40 and #43 across
provider-spec.md,message-flow.md, and NatSpec; addsReceiverReport.t.sol::test_aReportToAHomeBelowTheBarIsRefused. - Updates many stale doc claims (quotes, byte counts, section renumbering 4.6→4.5, C27/C19 test citations,
test/vectors/as planned). - Renames "msig" → "governor" in docs and
src/comments (bytecode unchanged).
| File | Description |
|---|---|
| contracts/evm/test/ReceiverReport.t.sol | New test proving a report to a home graded below the bar reverts and creates no account (verified correct, mirrors _wire). |
| contracts/evm/src/messaging/transceiver/TransceiverBase.sol | NatSpec: bar applies to every send/accept via _counterpartOn; msig→governor. |
| contracts/evm/src/messaging/outbound/OutboundBase.sol | _sendMessage @return clarifies zero/native-id semantics. |
| contracts/evm/src/messaging/inbound/ReceiverBase.sol | NatSpec on provider setup ordering relative to the guard/payload. |
| contracts/evm/src/messaging/Roles.sol, outbound/TransmitterBase.sol | msig→governor NatSpec wording. |
| docs/provider-spec.md | R1/R5.1/R6.3/R8.4, §3/§4.5, C1/C2/C19/C27, P9, checklist corrections. |
| docs/message-flow.md | 288-byte commit, inheritance diagram, Unknown-grade and bar wording. |
| docs/encoding.md | Marks test/vectors/ as planned; refreshes three code quotations. |
| docs/provider-research.md | __OAppSender_init, Core+Executor quote, OP Stack #41 notes, dropped phase labels. |
| docs/todo.md | Adds #41, names all five providers' default gas. |
| README.md | Arachnid/CrossProxyDeployer wording, governor, 703 tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fixes #40. Fixes #43.
#40, the provenance bar. The bar is checked in
TransceiverBase._counterpartOn, so it applies to every message a transceiver sends to or accepts from a chain: a bootstrap, a receiver report, an inbound delivery. ThebootstrapNatSpec, spec C19, andmessage-flow.mdnow say so. Adds the test the issue asked for,ReceiverReport.t.soltest_aReportToAHomeBelowTheBarIsRefused; disabling the bar check makes it fail.#43,
sendId. R1, the §3 override table, andOutboundBase._sendMessage's@returnnow say a native binding returns zero once the message is away and never its provider's own id; a binding over an ERC-7786 gateway passes that gateway'ssendIdon and completes or refuses a non-zero one.Read-through. Every doc read in full and checked against the code; each claim below was wrong or stale:
messageFeeplus ExecutorquoteExecution, not the deprecated Relayer); only LayerZero, CCIP, and Hyperlane price per byte (R2.3); native receivers funnel through_onMessageFrom(R3); provider setup runs before the reentrancy guard is initialized (R6.3, andReceiverBase's NatSpec); C1/C2 state what the tests assert; C27 cited a test that no longer exists; fork tests are C11 and C29 to C31; §4.6 renumbered.message-flow.md: a bare commit is 288 bytes, not 320; the inheritance diagram putReceiverBaseunderOutboundBase; the registry accepts anUnknowngrade and a bar ofUnknownpasses it.encoding.md:test/vectors/is planned, not built; three quotations no longer matched the code they cite.provider-research.md:LzTransmittercalls__OAppSender_init; internal phase labels dropped; the OP Stack per-pair rationale notes its reversal (OP Stack: one registry record per provider name allows one paired L2 per chain #41).todo.md: OP Stack: one registry record per provider name allows one paired L2 per chain #41 listed; all five providers' default gas named.CrossProxyDeployer(and forge's copy ofWormholeMessage) come from Arachnid's factory, accounts from their transceiver; the N × N source isdeploy/.governorOwner. Test identifiers keep their names.src/changes are comments only; the deployed bytecode of all 73srccontracts is identical with and without them. 703 tests; fmt, both lint profiles, and the size check pass.Not in this PR: the read-through found a bug in
ChainConfig.gradeOf(#46), fixed separately.