fix(mpp): bind session state to confirmed channels#239
Conversation
|
@greptile-apps please review the current head. |
|
@greptile-apps please re-review the current head after the confirmed-commitment fix. |
Greptile SummaryThis PR wires confirmed on-chain Channel account state into every MPP session open and top-up across TypeScript, Go, Rust, and Python. Confirmation alone no longer satisfies the security invariant — each SDK now fetches the Channel account at the confirmed slot, validates all PDA seeds (payer, payee, mint, authorized signer, salt, open slot, grace period, distribution hash), and overwrites the session's deposited amount with the authoritative on-chain value.
Confidence Score: 4/5The change substantially hardens the session open/top-up paths by binding persisted state to confirmed on-chain Channel accounts, but one cross-SDK validation gap remains: Go accepts N≥1 matching topUp instructions and sums their amounts, while TypeScript and Python reject anything other than exactly one. The four SDKs disagree on how many topUp instructions a single transaction may carry for a given channel — Go allows multiple and sums them, TypeScript and Python require exactly one. This is a live behavioural difference in a security boundary: a transaction crafted to satisfy Go but rejected by TypeScript/Python (or vice versa) produces inconsistent outcomes in mixed-SDK deployments. go/protocols/mpp/server/session_onchain.go (NewTopUpStateTxVerifier instruction-count logic) and python/src/solana_pay_kit/protocols/mpp/server/session.py (open_signature stored for all opens). Important Files Changed
|
…o fix/mpp-session-state-hardening # Conflicts: # typescript/packages/mpp/src/server/Session.ts # typescript/packages/mpp/src/server/session/on-chain.ts # typescript/packages/pay-kit/src/adapters/mpp-session.ts # typescript/packages/pay-kit/src/config.ts
|
@greptile-apps please re-review. The session settlement path now uses a durable transactional outbox, retry-safe exact-wire rebroadcast, lease recovery, expiry retirement, and construction-time RPC capability validation; focused Go/Python/Rust/TypeScript verification is green. |
|
@greptile-apps please re-review the final static-analysis head. The only follow-up changes simplify the always-present verified-open value and apply repository-mandated TypeScript key/import ordering; Go lint/full focused tests and TypeScript 568 tests/lint/format/typecheck are green. |
|
@greptile-apps please review |
Read the on-chain Channel account at the confirmed slot and bind session authoritative state (deposit, splits, grace period) to it. Session state is never advanced or trusted for an unconfirmed or economically mismatched channel. No sibling PR touches the Rust session server; this layer is unique to #239 and applies cleanly on the settled base.
Add #239's unique fail-closed binding: a session open/top-up is rejected unless the on-chain Channel account (fetched via getAccountInfo at the confirmed slot) confirms the claimed deposit, mint, payee, authorized signer, grace period and distribution hash, and a signature-only open is bound to its canonical broadcast transaction. - server/session/on-chain.ts: verifyChannelAccountState, verifySignatureOnlyOpenTransaction, GetAccountInfoRpc, TopUpTransactionRpc. - server/session/wire-tx.ts + store.ts: openSignature/salt/settlement fields carried on ChannelState for the binding. - server/Session.ts: wire the verify calls into open/top-up/routes; export SettlementRpc. - tests: session-state-binding, session-signature-only-open, plus the #239 additions to session-on-chain / session-server-on-chain / session-store. Dedup against stacked siblings #237/#238 (already in the base): keep their session-store durability guard (resolveSessionStore + allowUnsafeMemoryStore) and replay-store policy; drop #239's independent re-implementation of the same concern (requireProductionSessionSafety + allowUnsafeEphemeralStoreOffLocalnet + the adapter wiring), which conflicted with those siblings' regression tests. Superseded session-store-durability assertions were removed from the #239 test files; the mainnet fail-closed control is unchanged (stronger via #237).
Reconcile #239's on-chain Channel-account state binding onto the base's solana-go v2 migration and hardened open/top-up verification (#214). Adds the authoritative state-binding layer that no sibling or the base delivers: fetchAndBindChannelAccount, NewOpenStateTxVerifier, bindConfirmedOpenChannel, validateBoundOpenChannel, NewTopUpStateTxVerifier and confirmedTransactionSlot. Session open/top-up state is bound to the on-chain Channel account read at the confirmed slot; an unconfirmed or economically mismatched channel is rejected (fail closed). Base store-safety (env-gated) is preserved; the config-field guard reconciles to honor the same PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE opt-in. Ports #239's v1 solana-go usage to v2 and keeps base's structural verifiers + concurrency/replay hardening; drops #239's duplicate top-up-fence and legacy verifier redefinitions superseded by the base.
Add PR #239's unique behavior on top of sibling #228: the state-aware open verifier that fetches the on-chain Channel account at the confirmed slot and binds deposit, splits, grace period and PDA seeds to on-chain reality, so a payload's claimed economics are never persisted as facts. - session_onchain: BoundChannel, fetch_and_bind_channel_account, new_open_state_tx_verifier (+ signature-only confirm/bind helpers, _session_distribution_hash, _expected_session_grace_period). The structural open path reuses #228's verify_open_tx via the recipients contract. - confirm_transaction_signature now returns the confirmed slot and rejects a present-but-malformed slot, so the account read is pinned with min_context_slot; a status that omits the slot still confirms. - session: SessionConfig.verify_open_state_tx / open_tx_submitter seams, VerifiedOpenFacts, and process_open fail-closed: off localnet a payment-channel open requires the state verifier and persists only account-derived facts. - session_method: new_session installs the state verifier off localnet; _handle_open propagates the challenge recentSlot for signature-only push. - session_store: ChannelState.salt / open_signature round-trip. - rpc: SolanaRpc.get_account_info min_context_slot + typed get_transaction. - tests: add test_session_state_binding.py (adapted to #228's ProductionChannelStore policy, which is kept as-is). #228's structural open, top-up fence, channel-store policy and replay code are kept unchanged; their tests continue to pass.
Give the shared test operator a real signing key (sessionTestOperatorKey backs sessionTestRecipient) and co-sign the rentPayer slot so signature-only open fixtures reproduce a fully signed confirmed transaction. Base #214 verifies open signatures cryptographically, so the prior dummy-signature fixtures no longer pass. Also restores base's env-gated nil-store default (a merge artifact had re-added an unconditional off-localnet rejection that ignored the opt-in).
Base #214 hardened open verification (real wire-bound signatures, no placeholder acceptance) and the session store policy became env-gated. Update the stale Go test fixtures/assertions to the reconciled behavior: - flip TestNewOpenTxVerifierKeepsStructuralPlaceholderAcceptance to TestNewOpenTxVerifierRejectsPlaceholderSignature: the seam now rejects the pending placeholder ("must be a real transaction signature") - assert the real confirmed signature and derived PDA in the signature-only open test rather than the dummy reference the caller supplied - thread the derived channel PDA into the top-up test and reflect the post-top-up deposit in the authoritative on-chain account - match the reconciled off-localnet store-policy message, and enforce the unmarked-store rejection at the runtime safety gate (where it now lives) rather than at construction - rename the idle-close no-signer test and assert the no-op: closeOnIdle bails before touching the channel when no merchant signer is configured Test-only; no production source changed.
… path prepareOpenTx (SubmitOpenTx, the gasless server-submit flow) verified the open with allowUnsignedFeePayer=false, rejecting the placeholder fee-payer signature before the co-sign step the same function performs. The Go client signs only as the channel payer and leaves the operator slot for the server, so the server rejected its own client's open. Use the same verifyOpenTx(..., true) pre-verification handleOpen already uses: the payer signature is still verified and the post-co-sign zero-signature guard keeps broadcast fail-closed.
7ecf8e1 to
7012008
Compare
|
@greptileai review |
…resh test doubles confirm_transaction_signature returns int|None (a confirmed status may omit the slot), so _fetch_and_verify_signature_only_open's tuple[int, ...] annotation was unsound; the slot flows into min_context_slot (int|None), so widen the return. Refresh the _Rpc/_OpenConfig test doubles to the current RpcClient (search_transaction_history kwarg) and OpenVerifierConfig (settlement_window) protocols so pyright's structural check passes (13 errors -> 0).
…aths Add rejection-branch tests for validateBoundOpenChannel, resolveSessionTokenProgram, NewOpenStateTxVerifier/verifySignatureOnlyOpen, fetchAndBindChannelAccount, and the top-up verifiers (each asserts the concrete fail-closed error, not just coverage). Raises Go aggregate coverage 90.3% -> 91.1%, back over the 91.0 floor.
…ard for radar Prettier the mpp server index/session store re-exports. Mark the localnet-only MemoryChannelStore fallback nosemgrep: enforce_channel_store_policy above already rejects an absent/in-memory store off-localnet without the opt-in, which the radar rule's inline-guard pattern cannot see.
…ture The epoch-addressed channel PDA seeds include open_slot, so the session challenge now pre-fetches recentSlot alongside recentBlockhash and the open builder fails closed without it. The defect-verifier fixture predates that contract and crashed at construction instead of reaching its rejection assertions; supply the challenge slot so all five value-binding rejections (and the accept controls) exercise their intended paths.
semgrep only honors a nosemgrep comment on the match line or the line immediately before it; the wrapped two-line comment left the marker one line too far and the blocking radar scan still flagged the guarded localnet-only MemoryChannelStore fallback.
… union Order the mpp-session leaf (#239) ahead of the python leaf (#228) so their overlapping edits to the shared ci.yml + boot-policy.test.ts interleave without a phantom 3-way conflict (both are ultimately reconciled by the harness meta leaf, which owns those paths and merges last). Add a pure-union assertion after the merges: every non-merge commit the rehearsal introduces over base must be contained in some leaf head, else it is a hand-authored patch on #240 and the script refuses to push. This makes 'green by construction' mechanically checked, not asserted.
…into fix/mpp-session-state-hardening # Conflicts: # .github/workflows/ci.yml # go/protocols/mpp/server/session.go # go/protocols/mpp/server/session_method.go # go/protocols/mpp/server/session_method_test.go # go/protocols/mpp/server/session_onchain.go # go/protocols/mpp/server/session_onchain_test.go # typescript/packages/mpp/src/__tests__/session-server.test.ts # typescript/packages/mpp/src/server/Session.ts # typescript/packages/mpp/src/server/index.ts # typescript/packages/mpp/src/server/session/store.ts # typescript/packages/pay-kit/src/__tests__/mpp-session.test.ts
This leaf's session close decodes the signed wire to bind the broadcast signature before submitting, so the fixture's placeholder string no longer satisfies the contract. Bring forward the reconciled fixture: it compiles the composed instructions into a real signed transaction and asserts the reported signature is derived from the wire, never trusted from the RPC response. The gate travels with the hardening that created its need.
… into fix/mpp-session-state-hardening # Conflicts: # go/protocols/mpp/server/session_server_test.go # go/protocols/mpp/server/session_settlement_durability_test.go # go/protocols/mpp/server/session_state_binding_test.go # harness/test/value-binding-verify.test.ts # python/src/solana_pay_kit/protocols/mpp/server/session.py # python/src/solana_pay_kit/protocols/mpp/server/session_method.py # python/src/solana_pay_kit/protocols/mpp/server/session_onchain.py # python/src/solana_pay_kit/protocols/mpp/server/session_store.py # python/tests/test_session_method.py # python/tests/test_session_onchain.py # python/tests/test_session_server.py # python/tests/test_session_settlement.py # python/tests/test_session_state_binding.py # rust/crates/kit/src/mpp/server/session.rs # typescript/packages/mpp/src/__tests__/session-state-binding.test.ts
…tions The playground example still passed the removed allow_unsafe_ephemeral_store_off_localnet escape hatch to SessionOptions, so importing it (and collecting tests/test_playground_api.py) fails with a TypeError against this leaf's hardened session surface. Bring the example to the reconciled version, which constructs the session method without the ephemeral-store escape. Full python suite: 1518 passed, 1 skipped.
…into fix/mpp-session-state-hardening
…nto fix/mpp-session-state-hardening
…nto fix/mpp-session-state-hardening
…nto fix/mpp-session-state-hardening
…nto fix/mpp-session-state-hardening
…nto fix/mpp-session-state-hardening
Summary
Security invariants
Verification
server,clientfeature suite: 932 testsBase is #219. This is a protocol PR because the same payment-channel state-binding and durability contract spans multiple SDKs.