-
Notifications
You must be signed in to change notification settings - Fork 11
Remove legacy Ethereum OPP finalizer dependency #526
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,11 +35,10 @@ inline constexpr size_t SOLANA_MAX_CHUNK_BYTES = 672; | |
| * signature provider) with the outpost program id + IDL to implement the | ||
| * chain-agnostic SPI. | ||
| * | ||
| * The `deliver_outbound_envelope` implementation preserves the two-step | ||
| * Solana pattern: call `epoch_in` to stage the incoming envelope, then | ||
| * `emit_outbound_envelope` so the outpost emits any queued outgoing ones — | ||
| * the return value is the signature of the second call (the one that signals | ||
| * "work done for this epoch"). | ||
| * `deliver_outbound_envelope` stages chunks through `epoch_in`, then sends a | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consistency — this doc now says the inline emit replaces the separate call, but the plugin still binds it This rewrite (and the That binding is in a file this PR doesn't touch, so it's pre-existing — but since this doc change is what surfaces the inconsistency, worth either dropping the dead binding or noting why it's retained (recovery parity with the ETH side?). Non-blocking. |
||
| * zero-data terminal `epoch_in` call. When that call reaches consensus the | ||
| * program emits its queued outbound envelope inline; the return value is the | ||
| * terminal call's signature. | ||
| * | ||
| * Constructed by `outpost_solana_client_plugin::create_outpost_client` — | ||
| * `batch_operator_plugin` never builds one directly. | ||
|
|
||
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.
Test coverage — recovery
emitOutboundEnvelope(uint32)is never invoked or behaviorally testedThis retype (
emitOutboundEnvelope()→emitOutboundEnvelope(uint32)) is the PR's core code change, butemit_outbound_envelopehas no in-tree caller and the new tests only assert its ABI-encoding shape and that thestd::functionis bound (BOOST_CHECK(client->emit_outbound_envelope)). Nothing verifies that invoking it with auint32epoch actually produces correct recovery call data.Per
wire-sysio/CLAUDE.md: "Tests should verify behavior, not just that code compiles." Consider driving the recovery wrapper end-to-end against a mocked tx sink and asserting the epoch lands in the encoded args — or, if it is genuinely operator-tooling-only, a comment pointing at the out-of-tree caller. Non-blocking.