Skip to content

Add Openfort backend wallet as a keystore backend - #423

Open
joalavedra wants to merge 7 commits into
solana-foundation:mainfrom
joalavedra:feat/openfort-backend-wallet
Open

Add Openfort backend wallet as a keystore backend#423
joalavedra wants to merge 7 commits into
solana-foundation:mainfrom
joalavedra:feat/openfort-backend-wallet

Conversation

@joalavedra

@joalavedra joalavedra commented Aug 4, 2026

Copy link
Copy Markdown

What

Adds keystore: openfort accounts that sign remotely through Openfort's backend wallet API instead of holding a local keypair. The private key lives in Openfort's TEE and never touches the machine; pay stores only the project's API credentials (secret key + wallet secret) as a credential blob in the platform secret store, gated by the same Touch ID / Windows Hello / polkit path as local keypairs.

Because each signature is a policy-checked HTTPS call, an account can be constrained server-side (spend limits, allowed programs/mints) through Openfort's policy engine, and the local credentials are revocable — useful for agents that should never hold key material.

pay account new agent --backend openfort   # connect an existing backend wallet (acc_…)
pay setup --backend openfort               # same, as part of setup

How

  • pay-keystore: credential-blob storage under a credential: key prefix, with the existing auth gating. Blobs can never alias keypair entries, and the existing platform stores hold them unchanged.
  • pay-core (openfort module): credential storage and accounts.yml resolution around solana-keychain's OpenfortSigner (openfort feature). The signer builds the ES256 x-wallet-auth JWT, fetches and pins the wallet's Solana address at init, and verifies every returned ed25519 signature against it. Resolution cross-checks the address against the pubkey cached in accounts.yml. Raw-keypair access paths (pay account export, keypair loads) reject Openfort accounts with guidance.
  • pay-core (signer): the account-based loaders now return a ResolvedSigner enum (memory keypair or Openfort). Both MPP and x402 client paths already consume &dyn SolanaSigner, so they work with either. The string-source loaders (keychain:…, file paths) still return MemorySigner unchanged.
  • CLI: --backend openfort on pay account new / pay setup (interactive prompts, credentials validated against the API before anything is persisted, address cached in accounts.yml); pay account destroy removes the credential blob; import explains there is no keypair to import.

The [patch.crates-io] pin on solana-keychain

solana-keychain 1.4.0's openfort client builds its HTTP client with reqwest's ambient TLS default, which this workspace's Solana RPC crates force to native-tls — capped at TLS 1.2 on macOS, while api.openfort.io is TLS 1.3-only (solana-foundation/solana-keychain#225). The fix (solana-foundation/solana-keychain#234, pins rustls) is merged but not yet released, so the workspace patches solana-keychain to that commit. Drop the patch on the first release after 1.4.0. pay-kit doesn't forward the openfort feature, so pay declares a direct solana-keychain dep solely to enable it on the instance pay-kit re-exports.

Testing

  • cargo test --workspace --exclude pay-integration — all green; new unit tests cover credential-blob storage/gating, keystore serde, and signer routing. JWT/body canonicalization is covered by solana-keychain's own test suite.
  • cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --check clean.
  • Verified live against debugger.pay.sh with a real Openfort backend wallet (C78fUoBw1YDJDmzNx7viRZFnuhku3t3eiy9eiV2hafff):
    • MPP: 402 → build_credential (signed remotely by the TEE) → 200 with the quote payload — full end-to-end.
    • x402: the exact-scheme payment builds and signs identically to a local keypair. Decoding the built X-PAYMENT payload shows a well-formed transaction: slot 0 is the challenge's declared feePayer (left unsigned for the facilitator to co-sign) and the payer slot carries a signature that verifies over the exact message bytes. The debugger's devnet facilitator rejected the Openfort-signed and a control local-keypair payment with the same "did not pass signature verification" error — consistent with it simulating before co-signing its fee-payer slot, and in any case server-side and unrelated to the signer.

Try it locally (5 minutes)

The whole flow runs against pay server demo + the hosted Surfpool sandbox — no mainnet funds involved.

# Build (release embeds the web-ui debugger assets)
cd web-ui && pnpm install --frozen-lockfile && pnpm build && cd ../rust
cargo build --release -p pay

# Terminal 1: local paywall + payment debugger on :1402
./target/release/pay server demo

# Connect an Openfort backend wallet (prompts for the project secret key,
# a Solana backend wallet id `acc_…`, and the project's wallet secret)
./target/release/pay account new openfort --backend openfort

# Fund the wallet's address on the sandbox via Surfpool cheatcodes
RPC=https://402.surfnet.dev:8899; ADDR=<address printed above>
curl -s $RPC -H 'content-type: application/json' -d '{"jsonrpc":"2.0","id":1,"method":"surfnet_setAccount","params":["'$ADDR'",{"lamports":100000000000,"data":"","executable":false,"owner":"11111111111111111111111111111111"}]}'
curl -s $RPC -H 'content-type: application/json' -d '{"jsonrpc":"2.0","id":2,"method":"surfnet_setTokenAccount","params":["'$ADDR'","EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v",{"amount":1000000000},"TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA"]}'

# Pay: 402 → remote sign via Openfort → settle ($0.01 metered endpoint)
./target/release/pay --account openfort curl -s http://localhost:1402/api/v1/reports/usage

Watch the flow at http://127.0.0.1:1402; each payment also appears as a POST …/sign call in the Openfort project's API logs. Verified end-to-end on this branch: the $0.01 metered GET and the $1 checkout with payout split both settle, with and without the biometric gate (auth_required per network in accounts.yml).

🤖 Generated with Claude Code

Adds `keystore: openfort` accounts that sign remotely through Openfort's
backend wallet API instead of holding a local keypair. The private key
stays in Openfort's TEE; pay stores only the API credentials (secret key
+ wallet secret) as a credential blob in the platform secret store,
gated by the same Touch ID / Windows Hello / polkit path as keypairs.
Signing is a policy-checked HTTPS call, so an account can be constrained
server-side (spend limits, allowed programs/mints) and its credentials
revoked without touching a key.

- pay-keystore: credential-blob storage (`credential:` key prefix) with
  the existing auth gating, for secrets that are not 64-byte keypairs
- pay-core: `openfort` module with an `OpenfortSigner` implementing
  solana-keychain's `SolanaSigner` (ES256 `x-wallet-auth` JWT per
  request, hex payload signing, returned ed25519 signatures verified
  against the wallet's pinned address). Implemented in-tree rather than
  via solana-keychain's `openfort` feature because that client uses
  reqwest's ambient TLS default: the Solana RPC crates force native-tls
  into the build, which caps at TLS 1.2 on macOS, and api.openfort.io
  only accepts TLS 1.3. This client selects rustls explicitly.
- pay-core: account-based signer loaders now return a `ResolvedSigner`
  enum (local memory keypair or remote Openfort), so both the MPP and
  x402 payment paths work with either
- CLI: `pay account new <NAME> --backend openfort` / `pay setup
  --backend openfort` connect an existing backend wallet (validated
  against the API before persisting); destroy removes the credential
  blob; export and import reject Openfort accounts with guidance

Verified live against debugger.pay.sh: the MPP charge flow completes
end-to-end with a remote-signed credential (402 -> signed via Openfort
-> 200), and the x402 exact flow builds and signs its payment
identically to a local keypair.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

@joalavedra is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

Adds Openfort backend wallets as remote signers, stores their API credentials in the existing platform secret stores, and routes account-based payment, subscription, and server signing through the signer abstraction.

A destructive account-management failure was reproduced in pay account destroy --sandbox: when the same account name exists on both localnet and mainnet, the command keeps the localnet entry and removes the mainnet registration. This must be fixed before merge.

Confidence Score: 4/5

Not safe to merge until sandbox account deletion removes the selected network's account entry.

The reproduced deletion path can silently remove a same-named mainnet wallet registration while the user is deleting a sandbox wallet.

Files Needing Attention: rust/crates/cli/src/commands/account/destroy.rs

T-Rex T-Rex Logs

What T-Rex did

  • Attached a proof for the posted P1 finding and linked artifacts titled Focused Rust harness for duplicate localnet and mainnet account names and Successful sandbox destroy mutation harness output.
  • Produced a second proof for the posted P1 finding.
  • Executed the network-scope destruction validation using the Rust harness and observed the expected before/after state with localnet retained and mainnet account deleted, as shown in the after-state log.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. rust/crates/cli/src/commands/account/destroy.rs, line 161 (link)

    P1 Sandbox deletion targets mainnet

    --sandbox sets network to localnet and uses that entry for validation and credential cleanup, but this call unconditionally removes the same account name from mainnet. When both networks contain an account such as default, pay account destroy default --sandbox --yes leaves the localnet account configured and silently removes the mainnet registration. Remove from network here; the later default-account reassignment can remain explicitly scoped to mainnet.

    Artifacts

    Focused Rust harness for duplicate localnet and mainnet account names

    • This authored, compilable harness asserts the checked-in destroy statements and executes the command-relevant AccountsFile mutation with localnet and mainnet both containing NAME, proving the scope mismatch.

    Successful sandbox destroy mutation harness output

    • This captured command output shows the sandbox lookup selected localnet/NAME and the exact destructive call removed mainnet/NAME, confirming the defect.

    View artifacts

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 --sandbox account destroy deletes the same-named mainnet account

    • Bug
      • When both localnet/NAME and mainnet/NAME exist, pay account destroy NAME --sandbox --yes selects the localnet entry for validation and keystore work but persists deletion against mainnet. The focused executable harness proved the localnet entry remains and the mainnet entry is removed.
    • Cause
      • The command computes network as localnet for --sandbox at destroy.rs:31-35, but line 161 unconditionally passes MAINNET_NETWORK to AccountsFile::remove.
    • Fix
      • At rust/crates/cli/src/commands/account/destroy.rs:161, replace accounts.remove(MAINNET_NETWORK, &self.account); with accounts.remove(network, &self.account);. Keep default-account handling scoped to mainnet as appropriate.

    T-Rex Ran code and verified through T-Rex

Reviews (5): Last reviewed commit: "Format openfort signer error mapping" | Re-trigger Greptile

Rust 1.97's clippy rejects the redundant borrow in the hex-encode
format! argument under -D warnings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread rust/crates/core/src/openfort.rs Outdated
.build()
.map_err(|e| Error::Config(format!("Failed to create runtime: {e}")))?;

rt.block_on(OpenfortSigner::connect(credentials, account_id))

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 Nested runtime crashes Openfort signer resolution

build_signer unconditionally creates a Tokio runtime and calls block_on. When an async payment or server flow resolves an Openfort account while already running on Tokio, this panics with “Cannot start a runtime from within a runtime” before credential validation or an Openfort request. Make signer resolution asynchronous and await OpenfortSigner::connect, or safely isolate synchronous work outside the active runtime so callers receive an error rather than a process-level panic.

Artifacts

Focused Rust repro source for Openfort resolution inside and outside Tokio

  • Captured source of the executed public-API Rust test that compares normal synchronous resolution with resolution from an active Tokio runtime, proving the nested-runtime condition.

Openfort resolution control run outside Tokio

  • Executed control command shows the same Openfort resolution path returns its pre-HTTP configuration error outside an existing Tokio runtime, establishing the baseline.

Openfort resolution run inside Tokio runtime

  • Executed Tokio test catches the panic at openfort.rs:504:8 before Openfort connection or HTTP, confirming the failure hypothesis.

Openfort library test suite after focused repro

  • Executed existing Openfort-focused library tests show all seven current Openfort tests pass while the newly exercised nested-runtime path remains otherwise uncovered.

View artifacts

T-Rex Ran code and verified through T-Rex

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The panic is real tokio semantics, but the repro doesn't correspond to a reachable call path in this codebase — the sync-flow contract it trips over is pre-existing, and the suggested fixes would either break real callers or contradict the architecture:

  1. Every async call site already isolates these flows with spawn_blocking. The MPP/x402 builders this feeds have created internal runtimes and called block_on since before this PR (build_credential_with_override in client/mpp.rs, build_payment_with_override in client/x402.rs), so any direct call from an async worker panicked identically pre-PR. The payer proxy documents the contract explicitly (payer_proxy.rs: "select_challenge_by_balance / build_credential spin their own runtimes and may block on RPC + signing — keep them off the async workers") and wraps every payment build in spawn_blocking; the MCP tools wrap do_paid_fetch the same way before any signer resolution happens.

  2. A runtime-detection guard would break the legitimate callers. Handle::try_current() returns Ok inside spawn_blocking (the blocking pool inherits the runtime context), while block_on only panics on threads driving async tasks. Verified empirically against this branch: resolution returns a clean error on a plain thread and inside spawn_blocking, and panics only when called directly on an async worker — so a "detect and error" guard would false-positive on exactly the paths that are correct today.

  3. Asyncifying the resolution API would ripple through the whole synchronous account-loader surface and the CLI paths that call it, inverting the established design where sync flows own their runtimes and async surfaces isolate them.

Addressed by documenting the contract at the new entry point instead: load_openfort_signer now states it blocks on network I/O and that async callers must use spawn_blocking, same as build_credential/build_payment (90fa4fc).

joalavedra and others added 3 commits August 4, 2026 19:25
Openfort resolution blocks on network I/O via an internal runtime, like
the MPP/x402 payment builders it feeds; async callers must isolate it
with spawn_blocking, as the payer proxy and MCP tools already do.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pay account new registers Openfort accounts under mainnet only, so an
explicit --account on localnet/devnet silently shadowed the remote
signer with a lazy ephemeral of the same name. A remote signer signs
raw bytes and carries no chain state, so a same-named mainnet entry
with keystore: openfort now falls through to any network. Keypair-backed
accounts keep the old behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The server recomputes the JWT's reqHash over the key-sorted,
whitespace-free re-serialization of the body it parsed; any other
serialization fails with a bare 401 (verified against the live API).
Extract the body construction into sign_request_body, document the
contract, and add a known-answer test so adding a field forces a
conscious update.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
solana-keychain#234 (merged, unreleased) pins rustls in the openfort
client, fixing the ambient native-tls selection that made it unusable
in this graph on macOS (TLS 1.2 vs api.openfort.io's TLS 1.3-only).
Consume it via [patch.crates-io] until the first release after 1.4.0;
openfort.rs keeps only credential storage and accounts.yml resolution.

Verified live: 402 -> remote sign at api.openfort.io -> 200 on the
sandbox demo, payer signature checked against the backend wallet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@joalavedra

Copy link
Copy Markdown
Author

Update: the in-tree Openfort signer is gone. solana-keychain#234 (merged 2026-08-11) pins rustls in the openfort client, fixing the TLS issue from solana-keychain#225 that forced the vendored copy. c3f61a6 replaces it with solana-keychain's OpenfortSigner via [patch.crates-io] at the fix commit — net −335 lines; pay's openfort module now only owns credential storage and accounts.yml resolution. The patch line can be dropped as soon as a solana-keychain release after 1.4.0 ships.

Re-verified live on the sandbox demo: 402 → POST /v2/accounts/backend/{id}/sign → 200 with receipt, and the payer slot's ed25519 signature checks out against the backend wallet's address.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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