outpost_solana: read inbound envelopes through the IDL decoder

[valthon]

Problem

nodeop read the on-chain LatestOutboundEnvelope account with hardcoded Borsh offsets that assumed the standalone opp_outpost field order ({epoch_index, checksum, data, bump} — epoch_index at byte 8, past the 8-byte Anchor discriminator). The folded liqsol_core program declares {bump, epoch_index, checksum, data}, shifting epoch_index to byte 9, so the hardcoded reader decoded the leading bump (0xFF) as the low byte of the epoch (0xFF | 1<<8 = 511): read_inbound_envelope never matched the requested epoch, every SOL→depot envelope relay silently stalled, and no reward or deposit reached the depot.

Change

Read the account through libfc's IDL-driven decode_account_data — the same path this class already uses for Reserve / OutpostConfig. It verifies the 8-byte Anchor discriminator and follows the loaded IDL's declared field order, so one nodeop decodes both program layouts value-exactly and the hand-derived offset arithmetic (and its whole bug class) is gone.

Hardening around the read:

• IDL ↔ program-id match — the loaded IDL is selected by its declared address matching the deployed program id, so --solana-idl-file order can never decide which same-named IDL version's field order drives decoding; multiple candidates with none matching refuse to boot rather than silently pick one.
• Checksum validation — the stored keccak256(encoded_envelope) is verified, so a decode that read the wrong bytes for data (field-order drift) is caught and rejected instead of stalling.
• Owner check — accounts whose owner != program_id are rejected before decoding.
• Role-gated boot check — only roles that read inbound envelopes (batch_operator) require a decodable LatestOutboundEnvelope; the underwriter (which never reads inbound) is not bricked at boot by an instructions-only IDL.
• Drift signals (undecodable account, stored epoch ahead of request, checksum mismatch) log at warning level so a stalled relay is diagnosable; normal emit-cadence lag stays at debug.

Also retires the now-unused add_attestation binding + stub instruction: the folded liqsol_core IDL dropped it (attestations flow through epoch_in / emit_outbound_envelope), it had zero call sites, and binding it via get_idl("add_attestation") threw at construction against the folded IDL — nodeop now consumes the IDL verbatim.

Backwards-compatible: the standalone opp_outpost layout still decodes correctly (its field order drives the decode just as the integrated one does), and dropping the never-called add_attestation binding is inert for every program variant.

Verification

• Plugin unit tests green — test_outpost_solana_client_plugin (37 cases): the complete post-fetch read path driven end-to-end against synthesized accounts for both program layouts, the epoch=511 reproduction (integrated bytes read through a standalone-order IDL → rejected), every drift/reject path (bad discriminator, checksum mismatch, epoch-ahead, truncation, oversize, bad protobuf, owner mismatch), the IDL-selection rules, and a pin on libfc's bytes→base64 rendering the reader depends on.
• nodeop links (the solana_outpost_role threading through batch_operator / underwriter).

Companion PRs

• wire-solana (next-integ, ci: add independent macOS 25 workflow #385) — the folded liqsol_core outpost + --features development E2E build.
• wire-tools-ts (Wrong merge to sysio update #25) — harness support for the integrated outpost + real staking-yield flow.

🤖 Generated with Claude Code

https://claude.ai/code/session_015CbgvM8EhrPL1VjX8RhNze

[heifner]

Verified the offset derivation against both known layouts - the standalone order resolves epoch/vec_len/data to 8/44/48 and the integrated liqsol_core order to 9/45/49 - and that every subsequent buffer read stays bounds-safe (the data_off early-return plus the epoch-precedes-data assert cover the epoch and length reads). Also confirmed caller behavior: outpost_opp_job::run_inbound catches fc::exception, logs, and retries next cycle, so the new asserts fail loudly without taking down nodeop. Deriving from the IDL rather than adding a second hardcoded layout is the right fix.

Comments, roughly in priority order:

1. Test coverage: resolve_latest_layout and borsh_fixed_size are the heart of the fix but sit in the anonymous namespace with no tests. The plugin already unit-tests outpost_solana_client_detail functions (extract_inbound_recipient_pubkeys et al.), so moving these there enables table-driven cases: both field orders pinned to their exact offsets, a variable-length field before data throws, missing epoch_index/data throws. Given the RCA was wrong offsets silently stalling the relay, a regression test pinning the offsets for both IDLs is exactly the guard this change exists to provide.

2. Fields are located by name only. If a future layout declares epoch_index as u16/u64, read_u32_le misreads it; if data were a fixed [u8; N] there is no Borsh length prefix and vec_len_off would read payload bytes as a length. Two cheap asserts (epoch_index is u32, data is bytes/Vec<u8>) would complete the fail-loudly guarantee this PR is built around.

3. The RCA comment has the direction inverted: it says "the integrated offsets decoding the standalone account produced the epoch=511 RCA", but the removed constants hardcoded the standalone offsets (epoch at byte 8), and 511 = 0xFF | 1<<8 arises when those offsets read the integrated account (bump 0xFF at byte 8, epoch 1 at byte 9). The PR body states it correctly. Worth fixing since this comment is the postmortem record future readers will trust.

4. Design question, your call: the comment justifying per-read resolution notes the IDL is immutable after construction. Resolving once in the constructor would fail at boot on an undecodable IDL instead of at the first inbound poll (where the job loop wlogs and retries forever), surfacing operator misconfiguration much earlier. Per-read is defensible given the cost is negligible next to the RPC.

5. Minor: (*type.array_len) * (*element) in borsh_fixed_size is an unchecked multiply. Operator-file trust plus the downstream buf.size() checks make it low risk, but PR #500 in flight adds checked_mul_size in libfc for the same shape of computation. Related: borsh_fixed_size and #500's min_borsh_encoded_size look near-identical but are semantically different (exact fixed size vs conservative lower bound - string is nullopt in one, 4 in the other); a cross-referencing comment would keep someone from deduplicating them into a bug later.

[valthon]

Thanks Kevin — all five addressed in the follow-up commits.

1. Test coverage. Moved borsh_fixed_size + resolve_latest_layout (and the latest_envelope_layout struct) into outpost_solana_client_detail with header declarations, and added table-driven cases: both field orders pinned to exact offsets (standalone 8/44/48, integrated 9/45/49), a variable-length field before data, and missing epoch_index/data — each exercised through both the inline-account-fields path and the Anchor-v2 types-section fallback.

2. Field-type asserts. resolve_latest_layout now asserts epoch_index is primitive u32 and data is bytes/Vec<u8> (a fixed [u8; N], which has no Borsh length prefix, is rejected), with the offending declared type in the message. New cases cover u16/u64 epoch_index and [u8; 64] data → throw.

3. RCA comment. Fixed — it now reads the correct direction: the removed hardcoded standalone offsets (epoch@8) decoding the integrated account produced epoch=511 (bump=0xFF@8, epoch_index=1@9 ⇒ u32@8 = 0xFF | 1<<8), matching the PR body.

4. Resolve at boot. Took your recommendation — the layout is resolved once in the outpost_solana_client constructor (right after the program client is built), so an undecodable IDL throws out of create_outpost_client at boot instead of on the first inbound poll. The read site takes the cached member.

5. Checked multiply + cross-ref. The fixed-array width multiply is now overflow-checked — a local mirror of libfc's file-local checked_mul_size rather than widening its public surface. Added a note on borsh_fixed_size cross-referencing min_borsh_encoded_size: near-identical shape, but semantically different (exact-size-or-nullopt vs conservative lower bound, e.g. string→4) so they don't get deduplicated into a bug.

[heifner]

Reviewed at head 19083a3. The core approach is sound — I confirmed the integrated liqsol_core bump-first layout (epoch@9) genuinely exists on origin/opp-cleanroom-integrated (commit 8c0bca9, INITIAL_SIZE = 8+1+4+32+4), so deriving offsets from the IDL is the right fix for decoding both field orders with one binary. A few gaps worth addressing before merge.

Correctness

Offset accumulator can still wrap. resolve_latest_layout guards the per-field array width with checked_mul_size but the running off += *field_size sum is unchecked. A field like [u8; 2^64-8] passes the multiply, then the add wraps off to a small value, yielding bogus in-bounds offsets that pass every assert and the runtime buf.size() guard — the exact "wrapped offset" the checked_mul_size comment says it prevents. libfc's sibling min_borsh_fields_encoded_size already uses checked_add_size on the running total; mirror that here.

Layout binds to the first IDL, never matched to the program id. The layout resolves from get_program(), which libfc keeps as the first IDL in the vector, while the instruction map is last-wins — and idl::program.address is never compared against the client's program id. Two same-named IDL versions (realistic during the seed-rename churn the header notes) both pass the name filter, and the layout that wins is decided by --solana-idl-file order. If the first file's field order disagrees with the deployed program, this reintroduces the epoch=511 misread. Matching program.address to the program id would close it.

IDL-vs-deployment drift still stalls silently. This fixes the case where the operator's IDL file matches the deployed program, but if they drift, the misread lands in the stored_epoch != epoch_index branch, which is dlog-only — invisible at the default info level — and returns {} forever. Both layouts share the same name-derived discriminator and identical 49-byte size, and the checksum field (which exists precisely so the reader can cross-check) is never validated, so nothing distinguishes the drift at runtime. The original incident presented exactly this way. Worth at least promoting the mismatch log above debug, and ideally validating checksum.

Boot layout assert bricks roles that never read the account. The constructor requires a decodable LatestOutboundEnvelope for every role, including the underwriter, which never calls read_inbound_envelope. An instructions-only IDL (the shape of the in-repo stub fixture) that constructed fine pre-PR now throws — in the underwriter that becomes a terminal wiring_failed → app().quit() spanning all chains (ETH included); in batch_operator it's a per-refresh wlog+skip that also halts outbound epoch_in/deposit, contradicting the ctor comment's "throws at boot rather than wlog-retry-forever". Consider gating the layout resolution to roles that actually read inbound.

No discriminator or owner check on the fetched account. read_inbound_envelope decodes at derived offsets without verifying the 8-byte Anchor discriminator (which decode_account_data checks and resolve_latest_layout already has in hand) or account_info.owner == program_id. A program upgrade re-homing the seed would decode foreign bytes into an undiagnosable stall, or worse, hand coincidentally-matching bytes to ParseFromArray. Defense-in-depth — not the epoch=511 root cause (both layouts share the discriminator), but cheap insurance.

Reuse / altitude

The offset apparatus largely duplicates decode_account_data. borsh_fixed_size + resolve_latest_layout + the layout struct reimplement generic IDL-field-order-driven Borsh decoding that libfc already provides — and that this class already wraps as decode_account_info_data and uses for Reserve/OutpostConfig two functions away. read_inbound_envelope is the only account read that bypasses it with hand-derived offsets, which is what produced the field-order bug in the first place. Decoding the account per poll (15s cadence, 64KiB cap) would fix the bug value-exactly, delete the offset math + width table + checked_mul_size + layout struct + most of the new test lines, and pick up the discriminator check for free. The remaining need is a slim type-only boot assertion to preserve fail-loud and a small bytes-vs-Vec<u8> extraction helper. Also: checked_mul_size is a byte-for-byte copy of libfc's file-local one — per the "no duplicated helpers" invariant it belongs in a shared fc header (alongside checked_add_size).

Coverage

• No test constructs the client or drives read_inbound_envelope end-to-end; only the extracted helpers are unit-tested, and no fixture declares LatestOutboundEnvelope. The ethereum sibling ships this coverage (read_inbound_envelope_validates_latest_slot) — a case building both layouts' accounts and asserting the decoded epoch/data would catch real IDL drift and the first-wins selection.
• add_attestation removal is half-done: the client member and its boot binding are gone, but opp_outpost_has_initialize_and_add_attestation still asserts has_add and the stub fixture still declares the instruction.

Minor

• Field walk breaks at data, so an epoch-after-data order reports "missing epoch_index" for a field that exists; the epoch_off < vec_len_off assert is then unreachable for any non-wrapping input (only the unchecked off += can fire it, spuriously).
• is_primitive() then get_primitive() throws libfc's generic "Type is not a primitive" for an unrecognized type object (from_variant yields an empty-primitive idl_type{}) instead of the intended per-field diagnostic.
• data_off is derivable (vec_len_off + BORSH_VEC_LEN_PREFIX); the read-site layout alias + comment restate the member/ctor docs; the return std::nullopt after the exhaustive primitive switch is redundant.
• Test u8_32 is defined twice, and the inline-vs-types-section assertion block is a 2x copy a for (bool in_types : {false, true}) loop collapses.
• U+2014 em-dash inside an FC_ASSERT message string lands multi-byte UTF-8 in logs; a handful of comment glyphs likewise (the plugin already has pre-existing non-ASCII, so this is convention only).

[valthon]

Thanks — took the reuse direction. read_inbound_envelope now decodes the account through libfc's decode_account_data (the same decoder this class already uses for Reserve/OutpostConfig) instead of hand-derived offsets. I also reworked the PR so it presents the decoder approach directly rather than layering it on top of the offset code — so the offset-specific line comments won't map onto the current diff.

Per point:

• Reuse / decode (altitude): adopted. The reader follows the loaded IDL's declared field order, the discriminator check comes free, and the offset walk + width table + layout struct + checked_mul_size are gone. Cost was a non-issue in practice: data is declared bytes, so it decodes to a single base64 string (not a per-byte array), on a 15s-cadence poll.
• Offset accumulator wrap: moot — no offset arithmetic remains.
• First-IDL binding: fixed. The loaded IDL is selected by its declared address matching the deployed program id before the client caches it; multiple same-named candidates with none matching refuse to boot rather than pick by --solana-idl-file order.
• Silent drift: fixed. The reader validates the stored keccak256(encoded_envelope) checksum and logs undecodable / epoch-ahead / checksum-mismatch at warning level. One deliberate deviation: a merely lagging epoch stays at dlog — it fires every poll during normal emit cadence, and the ethereum sibling keeps its identical branch at debug for the same reason. Easy to split out if you'd prefer it louder.
• Boot assert bricks read-less roles: fixed. The boot-time shape check is role-gated — only batch_operator (which reads inbound) requires a decodable LatestOutboundEnvelope; the underwriter skips it. This adds a required role argument to create_outpost_client so every call site chooses explicitly — flag if you'd rather shape it differently.
• Discriminator / owner: the discriminator now comes from decode_account_data; added an owner != program_id reject before decoding.
• Coverage: the whole post-fetch path is factored out and driven end-to-end against synthesized accounts for both layouts — the epoch=511 reproduction, every reject path (bad discriminator / checksum / epoch-ahead / truncation / oversize / bad protobuf / owner), the IDL-selection rules, and a pin on libfc's bytes→base64 rendering so a future libfc change fails at test time rather than silently at runtime.
• add_attestation: finished — the stub instruction is removed and the test now asserts its absence.
• Minors: addressed. The idl_type::to_string() empty-optional hazard got a guarded describe_idl_type; the duplicate test helper and the inline-vs-types duplication are collapsed; no em-dash in the assert messages. I kept the trailing return std::nullopt after the primitive switch — the switch has no default, so removing it draws -Wreturn-type.

libfc is untouched.

Verified end-to-end — the full OPP flow suite (13/13) is green on both stacks, so the single nodeop binary reads both envelope layouts backwards-compatibly:

• integrated liqsol_core (next-integ + harness): https://github.com/Wire-Network/wire-platform-build-system/actions/runs/29967979848
• standalone opp_outpost default (this change only, on next): https://github.com/Wire-Network/wire-platform-build-system/actions/runs/29968295939