feat: make key registration a step in the invoice flow - #197
Draft
Atharva0506 wants to merge 5 commits into
Draft
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Atharva0506
force-pushed
the
feat/require-key-registration
branch
3 times, most recently
from
August 8, 2026 13:47
a19e10c to
5876950
Compare
The frontend ABI still described the pre-migration contract, which stored the full invoice payload on-chain as two strings: createInvoice(address, uint256, address, string, string) contracts/src/Chainvoice.sol has since moved to storing only a keccak256 commitment of the invoice data, and gained the public key registry: createInvoice(address, uint256, address, bytes32) Regenerated from `forge build` output so the ABI matches the contract source exactly. This also drops the `wakuPublicKeys` accessor, which is now a private mapping read through `getWakuPublicKey`. .env.example pointed at 0x54a5…, the old contract, where getWakuPublicKey reverts and getInvoice returns strings — following the setup docs would have produced an ABI/contract mismatch. Repointed at the Sepolia deployment of the current source. Ethereum Classic and Polygon still run the old contract and are flagged inline as needing redeployment. Callers in the invoice pages still pass the old string arguments and are updated separately; this commit only realigns the ABI with the contract.
Atharva0506
force-pushed
the
feat/require-key-registration
branch
3 times, most recently
from
August 10, 2026 12:51
07c62f1 to
6aff3f0
Compare
Waku supplied two things: end-to-end encryption and a transport. ThruBox is a dumb mailbox that supplies only the transport, so this adds both halves as an independent service layer: invoiceCrypto.js ECIES over secp256k1 (eciesjs), base64 framing relayKeyManager.js keypair derivation + on-chain key registry relayInvoiceMessaging.js send / fetch / poll of encrypted envelopes relayClient.js configured ThruBox client singleton invoiceHashUtils.js deterministic keccak256 commitment over payloads Key derivation is unchanged from the Waku implementation, deliberately: keys are already registered in the on-chain registry, and a different derivation message would silently break decryption for those users. The same 65-byte uncompressed public keys the contract already validates are what ECIES consumes here, so no contract change is needed. Three relay behaviours drove the design: - Address lookup is an exact string match, so a message sent to a checksummed address is invisible to a client polling the lowercase form. Every address crossing the boundary is normalised. - There is no cursor; each poll returns the whole mailbox. The poller tracks message IDs it has already surfaced rather than deleting from the relay, so a second device can still collect an invoice before it expires. - The SDK does not retry POSTs, since a retried send would duplicate the message. One timed-out send is therefore one undelivered invoice, so the timeout is configurable via VITE_RELAY_TIMEOUT_MS for hosts that suspend idle instances and cold-start slowly. The relay serves no CORS headers, so a browser cannot reach it cross-origin: the preflight for POST /api/messages is rejected outright and GET responses carry no Access-Control-Allow-Origin. A Vite dev proxy on /relay keeps development requests same-origin; production needs CORS on the relay or a reverse proxy in front of it. Nothing is wired into the invoice pages yet, so behaviour is unchanged.
Since Lit Protocol was removed, invoice payloads have been written to the chain as plain base64 — readable by anyone. This moves them off-chain: the contract now records only keccak256(payload), and the payload itself is ECIES-encrypted to the recipient's registered key and posted to the relay. Send (CreateInvoice, CreateInvoicesBatch): compute the hash, pass it as bytes32, then look up the recipient's key and post the ciphertext. Delivery is best-effort and never rolls back a confirmed invoice — a recipient who has not registered a key yet, or a relay that is briefly down, leaves the invoice on-chain and flagged as undelivered rather than failing the whole operation. Receive (ReceivedInvoice): payloads arrive over the relay into IndexedDB. Relay ingestion is its own effect, deliberately outside the refresh cycle: storing a message bumps a refresh trigger, and if that re-ran ingestion it would re-read the same mailbox and loop forever. Only invoices not already in IndexedDB count as new, since every poll returns the whole mailbox. Read (all pages): the payload is recomputed into a hash and compared against the chain before being trusted. This closes step 6 of the intended design, which the Waku implementation defined but never called anywhere. A payload that fails verification is not rendered; the invoice falls back to the on-chain amount and addresses, marked "Unverified", so a tampered payload can neither be shown as genuine nor hide the invoice entirely. Resend (SentInvoice): Waku's Store protocol retained messages network-side, so a client could always catch up. The relay only holds what was successfully posted, so an undelivered invoice would otherwise be stranded on the sender's device forever. Undelivered invoices now expose a resend action. The Pages deploy workflow gains the relay variables. Vite inlines env at build time, so without them a production build ships the localhost fallback, which the browser blocks as mixed content from an https origin. Verified end-to-end against a local relay and a deployed Sepolia contract: key derivation is stable, the payload re-hashes to the on-chain commitment after an encrypt/relay/decrypt round trip, non-recipients cannot decrypt, and altered payloads fail verification.
The registry stores a plain secp256k1 public key and says nothing about how the encrypted payload reaches the recipient. Naming it after Waku was accurate only while Waku was the transport; now it just misleads. registerWakuPublicKey -> registerPublicKey getWakuPublicKey -> getPublicKey wakuPublicKeys -> messagingPublicKeys WakuKeyRegistered -> PublicKeyRegistered InvalidWakuKey -> InvalidPublicKey BREAKING — requires redeployment. Renaming external functions changes their selectors, so no already-deployed Chainvoice answers to this ABI. The registry is contract storage, so a redeploy also starts it empty: every user has to sign and register again regardless of what this commit does. That re-registration is what makes it safe to also fix the derivation message, which still read "ChainVoice Waku Key Derivation v1". Since everyone must re-register anyway, changing it costs nothing extra, and leaving it would have kept a Waku reference in the one constant that can never be changed casually. Bumped to v2 to make the break explicit. .env.example is blanked rather than left pointing at 0x7bC4…, which no longer matches this ABI and would fail confusingly at the first call. Contract suite: 28 passed.
Registration was previously something you discovered by failing: create an invoice, then find out from a toast that nobody could read it. Both sides of that are now handled up front. Sender — registration comes before the form rather than after the transaction. The key lives in the on-chain registry, so a returning user is recognised from the chain and never sees the step; only a genuinely new address is asked to register. Registering is not cryptographically required to *send* — ECIES only needs the recipient's key — but it is required to receive, and an invoicing tool has no one-way users. `checkRegistration` now reports whether it is still checking. Without that, `isRegistered` is false during the initial read and the setup step flashes on every page load. A failed read still counts as unregistered: registering again is a no-op when the same key is already on-chain, so the cost of being wrong is one extra signature rather than a blocked or wrongly unlocked flow. Recipient — the registry is queried as soon as a valid client address is entered, so the sender is told whether details will actually reach them *before* paying gas, instead of after. An unregistered client no longer blocks anything: the invoice is still valid on-chain and still payable. Sent Invoices also sweeps for undelivered invoices on load and delivers any whose recipient has since registered. That is the case the manual resend button existed for, and it is entirely mechanical — the sender should not have to notice and click. The button stays for what a sweep cannot fix, such as a client who cleared their local storage.
Atharva0506
force-pushed
the
feat/require-key-registration
branch
from
August 10, 2026 13:23
6aff3f0 to
38c2045
Compare
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.
Addressed Issues:
Part of #139 (5 of 5). UX follow-up — the migration works without this, but the flow is rough.
Screenshots/Recordings:
Before: you created an invoice, then learned from a toast that the client could not read it.
After: registration is a step before the form, and the client's registry status is shown as soon as their address is entered — before any gas is spent.
Additional Notes:
Sender
Registration comes before the form rather than after the transaction. The key lives in the on-chain registry, so a returning user is recognised from the chain and never sees the step; only a genuinely new address is asked to register.
checkRegistrationnow reports whether it is still checking. Without that,isRegisteredis false during the initial read and the setup step flashes on every page load. A failed read still counts as unregistered — registering again is a no-op when the same key is already on-chain, so the cost of being wrong is one extra signature rather than a blocked or wrongly unlocked flow.Recipient
The registry is queried as soon as a valid client address is entered, so the sender is told whether details will actually reach them before paying gas, instead of after. An unregistered client does not block anything: the invoice is still valid on-chain and still payable.
SentInvoicealso sweeps for undelivered invoices on load and delivers any whose recipient has since registered. That is the case the manual resend button existed for, and it is entirely mechanical — the sender should not have to notice and click. The button stays for what a sweep cannot fix, such as a client who cleared their local storage.Also fixes a bug from #195
Keys were memory-only by default, so a page reload lost the private key and relay polling never restarted — the inbox silently stopped working. Keys now persist to
sessionStorage(still dying with the tab), the hook rehydrates them from cache without an extra signature, and polling keys off reactive state so registering or unlocking starts it immediately. 14 regression tests intests/services/relayKeyManager.test.jspin this.Review order: requires #193, #194, #195, #196.
AI Usage Disclosure:
Check one of the checkboxes below:
I have used the following AI models and tools: Claude Code (CLI), model Claude Opus 5
Checklist