fix(wallet): mint slash-free tiny.place identity keys#4779
Conversation
The tiny.place backend serves the Ed25519 identity public key as the
Signal bundle `identityKey` (STANDARD base64) and keys its
`/keys/{identityKey}/...` routes on it verbatim. A `/` in the base64
becomes `%2F` in the request path and the route 404s — so an identity
whose public key contains `/` can neither publish its Signal pre-keys
nor receive DMs. Our tiny.place identity is deterministic from the
Solana wallet key, so ~half of new wallets mint an unusable identity.
Derive a slash-free identity seed at the point we hand the seed to
LocalSigner:
- counter 0 == the raw wallet seed, so every already-clean identity is
byte-identical (the common case is unchanged, no migration),
- only when the raw key's base64 contains `/` do we deterministically
bump a domain-separated SHA-256 counter until the public key is
slash-free. Same wallet always maps to the same lowest-counter
slash-free identity.
Mirrors the workaround the tiny.place plugin already uses (regenerating
slash-free wallets); here we remap deterministically instead since the
identity must stay a pure function of the wallet key.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughSolana tiny.place signer seed generation now remaps seeds whose derived Ed25519 public-key Base64 contains ChangesSolana identity seed handling
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/wallet/chains/solana.rs`:
- Around line 733-744: Add a golden-vector assertion to the slash-bearing seed
test around slash_free_identity_seed, asserting the exact expected output seed
or derived public key for the fixed slashed input; retain the existing
slash-free and determinism checks to prevent accidental identity migration from
changes to hashing or encoding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb23741a-fd38-4538-a1c0-daeca88b22f8
📒 Files selected for processing (1)
src/openhuman/wallet/chains/solana.rs
| // Slashed seed is remapped to a slash-free, deterministic result. | ||
| let out = slash_free_identity_seed(slashed); | ||
| assert_ne!(out, slashed, "slashed seed must be remapped"); | ||
| assert!( | ||
| !pub_b64(&out).contains('/'), | ||
| "result pubkey must be slash-free" | ||
| ); | ||
| assert_eq!( | ||
| slash_free_identity_seed(slashed), | ||
| out, | ||
| "must be deterministic" | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Pin a golden remapping vector to prevent accidental identity migration.
Calling the helper twice only proves current-run determinism. Changes to the domain, counter encoding, or hash inputs would still pass while silently changing TinyPlace and Signal identities. Assert the exact output seed or public key for a fixed slash-bearing seed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/wallet/chains/solana.rs` around lines 733 - 744, Add a
golden-vector assertion to the slash-bearing seed test around
slash_free_identity_seed, asserting the exact expected output seed or derived
public key for the fixed slashed input; retain the existing slash-free and
determinism checks to prevent accidental identity migration from changes to
hashing or encoding.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 010c36441f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Never logged: the log below omits the seed. | ||
| log::debug!("[tinyplace] signer seed derived (seed not logged)"); | ||
| Ok(signing_key.to_bytes()) | ||
| Ok(slash_free_identity_seed(signing_key.to_bytes())) |
There was a problem hiding this comment.
For any wallet whose raw public key base64 contains /, this now returns a seed for a different Ed25519 key, so LocalSigner.agent_id() is no longer the Solana wallet address derived by the normal wallet path. The existing tiny.place payment and registry flows still treat signer.agent_id() as the wallet owner, e.g. wallet_usdc_balance(&signer.agent_id()), x402 from, and RegisterRequest.crypto_id, while execute_prepared spends from the original Solana wallet. In that slash-bearing case users can see/authorize under an unfunded or different cryptoId, and paid actions can be rejected after the on-chain transfer; keep the spending identity tied to the wallet and fix the /keys path/messaging key separately.
Useful? React with 👍 / 👎.
|
Closing this PR — re-diagnosis shows the fix is at the wrong layer, and is both unnecessary and unsafe. Thanks to @chatgpt-codex-connector for the P1 that prompted the deeper look. Why unsafe (Codex P1, confirmed)
Why unnecessary (root cause is elsewhere, and already fixed)The Every current send path already avoids this by fetching the bundle by base58 cryptoId:
The 404 I originally chased came from a stale SDK build in the observer prototype ( Correct follow-up (no OpenHuman change)
Closing. The recognition slice (#4775) and the observer findings are unaffected by this. |
…layer PR tinyhumansai#4779 (slash-free identity) was closed on review: the /keys/:cryptoId routes are base58-keyed and the backend resolves both encodings, so the 404 only occurs when a client puts the base64 identityKey in the URL path. Current send paths already fetch by base58 (TS SDK >=2.0.2 deriveCryptoId; OpenHuman resolve_recipient_to_agent_id -> crypto_id). The observer's 404 was a stale @tinyhumansai/tinyplace@1.0.1 build, not a product defect; remapping identity would have broken the payment/wallet coupling. Gap 1 reframed as the same base58 discipline, likely closed by an SDK bump — re-test before assuming an OpenHuman change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
slash_free_identity_seed()at the point the seed is handed toLocalSigner.Problem
The tiny.place backend serves the Ed25519 identity public key as the Signal bundle
identityKeyin STANDARD base64 and keys its/keys/{identityKey}/…routes on that string verbatim. Standard base64's alphabet includes/, which becomes%2Fin the request path — and those key routes 404 on it. So an identity whose public key's base64 contains a/cannot publish its Signal pre-keys and cannot receive DMs.Our tiny.place identity is deterministic from the Solana wallet key, and roughly half of all Ed25519 public keys contain at least one
/in standard base64. So ~half of new wallets mint a silently-unusable tiny.place identity (bundle publish + inbound DM both 404). This is the same class of failure theplugin-tinyplaceSDK already sidesteps by regenerating slash-free wallets.Solution
tinyplace_signer_seed()now returnsslash_free_identity_seed(signing_key.to_bytes()):/do we deterministically derive a fresh seed fromSHA-256("openhuman.tinyplace.identity.slashfree.v1" ‖ seed ‖ counter), bumpingcounteruntil the resulting public key is slash-free. Same wallet always maps to the same lowest-counter slash-free identity.The tiny.place plugin regenerates slash-free wallets; here the identity must remain a pure function of the wallet key, so we remap the seed deterministically instead.
Submission Checklist
slash_free_identity_seed_cleans_slash_keys_deterministicallycovers the clean-passthrough path (counter 0, unchanged) and the slashed→remapped path (slash-free + deterministic).fn+ its unit test are fully exercised (both branches). Flagging for the coverage gate to confirm.N/A: internal key-derivation fix, no user-facing feature row.N/A, uses already-importedsha2/ed25519_dalek/base64.N/A: no release-cut surface touched.Impact
debuglog omits the seed)./, this changes that wallet's advertised tiny.place identity to a different (slash-free) key. In practice those identities were already non-functional — they 404 on bundle-publish and on inbound DMs — so there is no working relay state to preserve, and remapping is strictly a repair. But if any such identity was partially advertised (e.g. published out-of-band, printed in a UI, or saved by a peer), those references will no longer resolve after upgrade. Recommend the team confirm no persisted slash-bearing identities need an explicit migration/announcement before merge. Clean identities (counter 0) are unaffected.Related
AI Authored PR Metadata
Linear Issue
Commit & Branch
fix/tinyplace-slash-free-identityValidation Run
pnpm --filter openhuman-app format:check— N/A (noapp/changes)pnpm typecheck— N/A (no TS changes)cargo test -p openhuman --lib slash_free_identity_seed_cleans_slash_keys_deterministically→ 1 passedcargo fmt --check -- src/openhuman/wallet/chains/solana.rs→ cleanBehavior Changes
Parity Contract
Summary by CodeRabbit