Implement provider interfaces, enforce transfer state machine, and KYC policy - #4
Merged
Merged
Conversation
Phase 0.1 of Docs/progress.md. Names the two things Prova does — proving a transfer legitimate, and settling value — so the application depends on what is done rather than on who does it. SettlementProvider moves value the privacy layer authorised PrivacyProvider serves the public state a wallet cannot see itself StellarSettlement and StellarPrivacy adapt the existing relayer and pool service. Same code underneath, same behaviour: this is a boundary, not a rewrite. poolSpend is the first route migrated and its responses are byte-identical, verified against the existing diagnostic-leak guard. Typed errors replace string matching. The caller's decision differs per case — retry, re-prove, report success, page someone — and deciding that by matching error text is how a definite failure once got reported as "still processing" because CLI output contained the word "network". Retryable() defaults to false for anything unrecognised: a wrong "true" resubmits a payment. Idempotency is by nullifier, not bookkeeping. The contract refuses a nullifier it has already seen, so a duplicate submission cannot move value twice — enforced on-chain rather than in one replica's memory. Providers are held alongside the pool service rather than replacing it. Routes migrate one at a time; swapping every call site at once would risk a working product for a refactor. Also adds the V2 architecture review and the phased delivery plan the work follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0.2 of Docs/progress.md.
The seven states already existed in schema.TransferStatus, but only as a
comment describing the intended order — nothing checked it. Any code
holding a transfer could set any status, so "confirmed" could follow
"failed" and a terminal transfer could quietly reopen.
For a payments system the status is what tells someone whether their
money moved, so this is now a table with an exhaustive test over all 7x7
pairs. The rules that matter:
- confirmed never goes backwards. Once the chain records it, no later
event may claim it did not happen.
- terminal states stay terminal, so a late duplicate cannot reopen a
finished transfer.
- self-transitions are legal, because at-least-once delivery is normal
and a machine that fights redelivery is one every consumer works
around.
- Settled() is deliberately narrower than Terminal(): rejected and
failed are finished, but nothing moved.
Store.SetStatus enforces it inside a transaction with SELECT ... FOR
UPDATE. Validating in Go against a previously-read status is the
check-then-act race that lets two writers walk a transfer backwards; the
row lock makes the loser read the winner's result and be refused.
One correction to my own table: it originally forced
submitting -> submitted -> confirmed, which would have rejected every
transfer the working relayer makes. The Soroban submitter returns only
once the chain has accepted, so a successful submit IS the confirmation.
submitted remains for an asynchronous submitter. Verified by asserting
the exact sequences internal/transfers/service.go performs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s and transfer state machine
Phase 0.3 of Docs/progress.md, option B.
MIN_KYC_LEVEL moves from FpVar::constant to a public input supplied by
the contract from its own storage. A corridor can now raise the bar with
one admin call instead of a new circuit, a new trusted setup and a forced
app update.
Public input, not witness: a witness would let the prover choose its own
minimum and prove kyc_level >= 0. Appended LAST (index 15) so indices
0..14 keep their meaning — the verifying key's IC layout follows
allocation order, and renumbering an existing input breaks every proof
with no diagnostic beyond "rejected".
Three properties proven in the circuit tests:
- a level-2 credential satisfies 2 and fails 3
- substituting the minimum after proving invalidates the proof, in both
directions. Without this the input would be unbound and a wallet
could pick its own bar
- changing the policy does NOT change the constraint count or the
verifying key, which is the entire point
Contract: DataKey::MinKycLevel, public min_kyc_level(), admin-only
set_min_kyc_level() emitting an event — modelled on set_anchor. Missing
policy reads as DEFAULT_MIN_KYC_LEVEL so an older pool is unchanged.
Enforced by the pairing check, so the failure mode is InvalidProof, the
same as a rotated anchor: no separate branch to bypass.
The wallet reads the policy from /pool/status rather than assuming it —
proving against a stale value fails exactly like a stale root. The
backend reads it from the contract with a 60s cache and serves a stale
value over none.
REDEPLOYED, unavoidably: 15 -> 16 public inputs changed the verifying
key, so the old contract cannot verify new proofs.
old CBLLKIUUWPH4GCPL4NNK6S6NGDG4OEAX33TTYJ7RPO3SZU52FHYYJEVX
new CD645P75NWNDIYZZ3363ABNK6WL435KLYOQYKUPLHLMEXIF7QMI5WGUA
Every reference updated: README (4), handoff-linux, backend/.env,
backend/.env.deploy. DEPLOYMENTS.md records the new pool as current and
keeps the old one as superseded. The four historical transactions stay
listed against the contract that executed them.
Also fixes a real bug in the deploy script: it warned when ANCHOR_SEED
was unset but then called anchor-pubkey WITHOUT passing it, so exporting
the seed changed nothing and the pool initialised with the built-in dev
key. Caught by reading AnchorPk off the chain after deploying rather than
trusting the script; rotated to the backend's key (tx 29dbe406).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0.4 of Docs/progress.md.
Idempotency was already sound, and the audit is worth recording: a retry
carries the same nullifier, the contract refuses one it has seen, the
provider maps that to ErrAlreadySettled (a success that already
happened, not a failure), and store.Create returns the existing row via
ON CONFLICT (nullifier). Enforced on-chain rather than by bookkeeping,
which is stronger — the guarantee holds across replicas rather than
living in one process's memory.
Reconciliation was the real gap. Every health signal is healthy-by-
default: the API answers, the folder folds, the queue drains. A transfer
stalled between submitting and any outcome appears in none of them — one
row nothing will move again, where the only person who finds out is
whoever was owed the money.
Store.StuckTransfers non-terminal and not updated within a window,
oldest first
GET /ops/reconcile behind COMPLIANCE_TOKEN like every /ops route
Each row reports `settled` alongside the status, because that is the
distinction an operator needs first: a stalled transfer that already
settled is bookkeeping, one that never settled is a missing payment.
The query's state list is asserted against lifecycle.InFlight rather
than derived from it. Deriving would make them agree by construction and
prove nothing; comparing two independent statements catches the case
where someone adds a state to the machine and forgets the query, which
is exactly how reconciliation goes blind.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0.5 of Docs/progress.md, in part. RecordRelayFailure was write-only, so one bad relay left an error on /pool/status permanently — it still reads "proof rejected: Error(Contract, #4)" from 24 August, after the cause was fixed and every send since has worked. That is worse than reporting nothing. An operator learns the field is noise and stops reading it, and anyone else clicking the endpoint sees a healthy pool describing itself as broken. A status field that only ever goes one way is not a status field. ClearRelayFailure runs on a successful spend, best-effort and after the response is decided: failing to clear a stale message must never fail a transfer that already succeeded. The two remaining 0.5 items need things I cannot do — a device to recapture payment_failed.png, and a browser for a mobile-width site screenshot. Both are recorded in the tracker with why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Carries the Phase 0 work that reaches the app: 65d2b71 verification submit names the real failure 52504b4 survives slow and congested networks a24e487 configurable KYC policy — the wallet now reads the minimum from /pool/status and proves against it The last one is why this cannot share a version with 1.2.7. The spend circuit went from 15 to 16 public inputs, so a 1.2.7 build cannot produce proofs the new pool (CD645P…) accepts, and a 1.2.8 build cannot produce proofs the old pool accepts. The version is the only thing distinguishing two APKs whose difference is invisible until a send fails. No APK built for this one: testing now happens over USB against a local build. The version still moves so the installed build is always identifiable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two bugs found while building locally. Both lived in the generated android/ directory, which is gitignored — so a hand fix there would be lost on the next prebuild and never existed for EAS or a teammate. Added to the config plugin instead, which is what that plugin is for. 1. armeabi-v7a was built despite buildArchs `buildArchs` in app.json and `reactNativeArchitectures` in gradle.properties both constrain React Native's own build. Expo modules with their own CMake — expo-updates, react-native-worklets — read neither, and build every ABI the NDK offers unless the app module declares abiFilters. So the build spent minutes compiling armeabi-v7a and then failed on it, for an architecture this app deliberately does not ship: the Rust prover has no 32-bit build, so a v7a device would install and then fail at the moment it tried to send. Note the arm64 variant of the same task succeeded — only v7a broke. 2. versionName was hardcoded 1.0.0 The generated build.gradle froze versionCode 1 / versionName "1.0.0" on 17 Aug. Prebuild normally rewrites these, but a checked-out android/ that predates a version bump keeps the stale values — so a local build installed as 1.0.0 while app.json said 1.2.8, and the installed build could not be identified. That is the one thing a version number exists to do. versionCode is now derived from versionName (1.2.8 -> 10208) so it rises with it: Android refuses to install over a higher code, and a frozen 1 makes every build look identical to the package manager. Verified by running both transforms against the real generated shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1.1 done, 1.2 half done. Measured the baseline rather than estimating it (report_circuit_sizes, report_proving_times): spend 24,729 constraints 16 public inputs ~700-800ms desktop shield 6,684 constraints 7 public inputs ~215-240ms Timings are ranges across runs, not point values — the spread on this machine is ~15%, and quoting one run to the millisecond would imply precision that is not there. Any Midnight comparison has to clear that noise floor to mean anything. The finding: the proposal's strongest argument was policy flexibility — a compiled-in constant meant a corridor could not change its rules without a new circuit and a forced app update. Phase 0.3 answered that without Midnight, and the same technique extends to any other scalar policy while staying in Groth16. What remains is one real limitation (1-in-2-out spends, which users already hit as "your balance is split across notes") and it is a circuit change, not a platform change. The one property that would justify switching platforms is eliminating the trusted setup, and nobody has confirmed Midnight does that. The Midnight column is left empty on purpose. Filling it by reading documentation would repeat the exact failure this phase exists to prevent: the V2 proposal asserted Midnight was "specifically suited to programmable privacy" without a single measurement. Recommendation: do not start Phase 2. Build 2-in-2-out spends instead — the one named user-facing limitation, achievable on the current stack, and it would make any future comparison sharper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.