Skip to content

feat(bench): enable sponsored session opens - #430

Open
lgalabru wants to merge 4 commits into
feat/pay-bench-pingorafrom
feat/pay-bench-fee-sponsorship
Open

feat(bench): enable sponsored session opens#430
lgalabru wants to merge 4 commits into
feat/pay-bench-pingorafrom
feat/pay-bench-fee-sponsorship

Conversation

@lgalabru

Copy link
Copy Markdown
Collaborator

Summary

  • pin the draft-00 PayKit session implementation from PR feat!: migrate @solana/pay to @solana/kit #283
  • wire the configured operator signer into session fee sponsorship
  • run the devnet benchmark with fee_payer: true
  • use post-co-sign transaction IDs and mint-normalized session dispatch

Verification

  • cargo fmt --all -- --check
  • cargo clippy -p pay-bench -p pay-core -p pay --all-targets -- -D warnings
  • cargo test -p pay-bench -p pay-core -p pay --no-fail-fast (1,388 passed; 1 ignored)

Stacked on feat/pay-bench-pingora so it includes the bounded fixture executor.

Co-authored-by: Ludo Galabru <ludo.galabru@solana.org>
Signed-off-by: Ludo Galabru <ludo.galabru@solana.org>
Co-authored-by: Ludo Galabru <ludo.galabru@solana.org>
Signed-off-by: Ludo Galabru <ludo.galabru@solana.org>
Co-authored-by: Ludo Galabru <ludo.galabru@solana.org>
Signed-off-by: Ludo Galabru <ludo.galabru@solana.org>
Co-authored-by: Ludo Galabru <ludo.galabru@solana.org>
Signed-off-by: Ludo Galabru <ludo.galabru@solana.org>
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
payment-debugger Ready Ready Preview Aug 12, 2026 10:46am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown

Greptile Summary

This change improves benchmark fixture recovery by journaling pending setup and teardown operations, but a confirmed transaction failure leaves its pending journal entry in place. Every later resume then stops on the same terminal failure, preventing automated reconciliation or cleanup until an operator manually alters the journal.

Merge safety: do not merge until terminal transaction failures have a durable acknowledgement or recovery path.

Confidence Score: 4/5

The pending-transaction recovery path is unsafe for confirmed transaction failures because it cannot progress beyond the retained terminal journal record.

A focused executable test exercised the recovery method twice against a local RPC response containing a confirmed instruction error. Both attempts failed on the same journal entry, directly confirming the affected behavior.

Files Needing Attention: rust/bench/src/fixtures.rs needs a durable terminal-failure handling policy and regression coverage for repeated resume attempts.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex executed a focused Rust harness to reproduce a confirmed failed pending transaction.
  • T-Rex inspected the focused test output that shows a failed pending record persists across two resumes, confirming the posted finding.
  • T-Rex completed a general-contract-validation-proof by running the pending_reconciliation_regression test with cargo and verified the observed failure and restoration behavior.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Confirmed failed fixture transactions permanently block resume

    • Bug
      • When getSignatureStatuses returns a confirmed status containing err, resolve_pending returns an error before clearing the matching FixtureJournal pending record. Every subsequent setup or teardown resume processes and fails on that same terminal record before reconciliation can proceed.
    • Cause
      • The status.err branch at lines 908-914 calls bail! directly, while journal.clear_pending(&transaction.signature) is only executed on the successful-status path at line 915.
    • Fix
      • Decide and document the recovery policy for terminal transaction failures. If a failed transaction should be acknowledged to permit idempotent reconciliation/rebuild, persistently clear or mark the pending record as terminal before returning the error; add a regression test covering two resumes. If retaining it is intentional, provide an explicit durable recovery/acknowledgement path rather than making all resumes fail indefinitely.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat(bench): enable sponsored session op..." | Re-trigger Greptile

Comment on lines +907 to +914
if let Some(status) = rpc.signature_status(signature).await? {
if let Some(error) = status.err {
bail!(
"pending {} transaction {} failed: {error:?}",
transaction.operation,
transaction.signature
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Failed transaction remains pending

When a submitted setup or teardown transaction has a confirmed error, this branch returns before removing or terminally acknowledging its pending journal record. Each subsequent resume reads the same record and fails again before reconciliation can continue, leaving funded fixture operations unable to recover without manual journal intervention. Persistently clear or mark the entry terminal before returning the error, and add coverage for repeated resumes.

Artifacts

Executed focused Rust harness for a confirmed failed pending transaction

  • Temporary Rust test source uses a local JSON-RPC server that returns a confirmed transaction error and invokes reconciliation twice; it exercises the exact pending-journal path and was removed from tracked source afterward.

Focused test output showing a failed pending record remains across two resumes

  • Captured output from the executed cargo test shows both reconciliation calls return the terminal transaction error while the pending journal count remains one after each call, proving the resume loop.

View artifacts

T-Rex Ran code and verified through T-Rex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant