Consolidate extras into main (unify repo ahead of public release) - #11
Merged
Merged
Conversation
Add OZ Ownable admin (owner set in constructor) plus allow/disallow/ is_allowed backed by the stellar-tokens AllowList extension. Gate deposit on the share recipient and SEP-41 transfer/transfer_from on both sender and recipient.
Regenerate the vault TS client against the admin=gov deployment (CAKZLKMWWVFUAGGVGX5EI6F3AOZEORSNM4D2POBSU7DXD3ISRXIIVW42) so it exposes allow/disallow/is_allowed instead of the stale token-only bindings, and pin that contract id as a named config constant for the upcoming admin flow. Add vitest scoped to app/src/lib for pure-logic unit tests; no React/DOM setup needed. The stellar-sdk version drift turned out to be a stale node_modules state (orphaned nested installs under app-lib/clients/*), not a wrong declared range — reinstalling resolves ^16.0.1 consistently everywhere.
Add three vitest-covered, I/O-free modules under app/src/lib that back the upcoming admin signing flow: - sep7.ts: encode/decode the /admin#tx=<base64url xdr> transport, plus a web+stellar:tx?xdr= URI for SEP-7 fidelity. Rejects malformed or missing tx parameters without attempting a partial decode. - threshold.ts: computes collected multisig weight by verifying each known signer's key against the envelope's signatures (via the tx hash), so duplicate signatures and non-signer keys never inflate the weight. - txSignatures.ts: merges two envelopes' DecoratedSignature[] sharing the same transaction body, preserving prior signatures and deduping byte-identical ones. All three are covered by colocated *.test.ts files built test-first (RED before implementation), 20 tests total, all passing.
Add adminTx.ts (build->simulate->assemble via rpc, plus decode/submit) and useAccountSigners.ts (Horizon signers/thresholds) so the admin page can compose and score multisig signature rounds. Fix computeCollected to skip non-ed25519 Horizon signers (preauth_tx/sha256_hash) instead of crashing on Keypair.fromPublicKey, caught by a new RED test first.
Add /admin route with Compose (build+simulate+assemble, emit a shareable /admin#tx= link) and Sign-round (decode, display op/target/ source/collected weight before signing, sign+merge, re-encode) views. Submit is gated on collected weight >= med_threshold. Signature merging always runs client-side via mergeSignatures regardless of whether the wallet appends or replaces prior signatures, so the flow does not depend on that unverified wallet behavior. Manual verification still required: a full 2-of-3 browser round-trip with Freighter (compose -> sign with signer A -> share link -> sign with signer B -> submit -> confirm is_allowed(target) flips) has not been run. To run it: start the dev server, connect Freighter as one of the gov signers, compose an allow op for a test address, copy the /admin#tx= link, open it in a second session as signer B, sign both rounds, and submit once the Submit button enables.
submitAdminTx only checked the RPC send status, so the UI could report "allowlist updated" for a tx that was merely queued and later failed. Now poll getTransaction until SUCCESS (or fail fast on FAILED/timeout).
- Compose now redirects to the sign view so the composer adds the first signature directly, instead of surfacing a link to hand to themselves. - After submit, show a success screen with the tx hash and an explorer link; the sign/submit controls are gone, preventing repeat submits.
Show the Admin nav entry and admin console only when the connected wallet is a signer of the vault's gov account. UI-surface gate only; on-chain require_auth against gov's thresholds remains the real authority.
- Pin the address chosen at connect; the 1s poll no longer re-queries the wallet's active account, which made the dapp flicker between multiple connected accounts. Switching accounts now requires an explicit reconnect. - Call the latest updateCurrentWalletState from the poller via a ref so its state comparisons use current values, not the stale mount-time closure. - Don't sign the user out on a transient getAddress/getNetwork error.
- Gate mint on the receiver's allowlist status, closing a KYC bypass: mint is deposit's sibling (both create shares) but was ungated. - Extend instance-storage TTL at the start of every mutating entrypoint; the contract must keep its own instance entry alive (OZ handles the rest). - Add vault unit tests covering the allowlist gate on deposit/mint/transfer and owner-only allow/disallow (9 tests).
- Block the deposit tab with a KYC message when the connected wallet is not allowlisted (is_allowed via useVault); withdraw stays open so a de-listed holder can still exit. - Point the generated client and adminConfig at the redeployed hardened vault (CDBJBS3T...) with the mint gate + instance-TTL extension. - Drop the unused SEP-7 web+stellar: URI field from the admin link codec. - Update AGENTS.md: KYC allowlist + gov multisig owner + new deployment id.
Maintain an instance-storage counter of addresses currently holding shares (>0), adjusted on every balance transition across deposit/mint/withdraw/ redeem/transfer/transfer_from (0->+ increments, +->0 decrements). Exposed via lp_count(). 8 tests covering the transitions.
Read lp_count() in useVault and surface it as a 'Liquidity providers' stat on the dashboard; repoint the client and adminConfig at the redeployed vault (CD5RPBZ6...) that tracks current holders.
Reflect the KYC allowlist, 2-of-3 multisig owner + SEP-7 admin console, the on-chain LP counter, and the current deployment (CD5RPBZ6). Add a features section, the admin-console flow, and the vitest/npx-vite notes.
Drop scaffold boilerplate (packages/, src/contracts/ stubs, /debug, generic agent-skills section) and the stale "no automated tests" note. Document the actual layout (pages/components/hooks/lib/providers), routes (/ + /admin), vitest coverage, the pinned-account WalletProvider, and client regeneration.
- vercel.json: rewrite all routes to index.html so /admin (and any deep link or refresh) resolves via react-router instead of 404ing on the static host. - useVault: keepPreviousData + retry so a transient public-RPC hiccup doesn't blank the dashboard (fixes the flicker) or drop derived state. - Split is_allowed into its own small resilient query keyed by address, so a failure in the heavier vault reads never hides the deposit form; allowlist status stays sticky across refetches.
Drive the skeleton off the absence of data (!data) instead of react-query's isLoading, which could turn true on background refetches and flash the skeleton each polling cycle.
Root cause of the 'not allowlisted' bug: the auto-generated @stellar-scaffold/app-lib/clients wrapper baked the OLD pre-allowlist vault id (CARQ5UVS) into its `vault` singleton, so the dashboard read one contract while the admin console wrote to CD5RPBZ6 — two different vaults. Add app/src/lib/vaultClient.ts: a client pinned to ADMIN_VAULT_CONTRACT_ID (the single source of truth), used by useVault/useIsAllowed and the dashboard, so a scaffold regeneration can never make them diverge again. Also add a @/* -> src/* alias (tsconfig paths + vite resolve) and switch the touched files to absolute imports.
- Guard dashboard signing on wallet-network mismatch (enableUsdc/deposit/ withdraw), mirroring the admin page — root cause of the trustline op_bad_auth. - Surface Horizon result_codes instead of opaque axios messages (new errors.ts helper), in the trustline + deposit/withdraw flows. - Show tx hash + explorer link on deposit/withdraw success. - Use rpc.pollTransaction in submitAdminTx instead of a hand-rolled loop. - Stop background/hidden tabs polling (refetchIntervalInBackground: false). - Rename sep7.ts -> adminLink.ts (bespoke #tx= hash transport, not a SEP-7 URI); update test, imports, and docs. - Validate PUBLIC_STELLAR_*_URL as URLs (zod z.url()); note pinned ids are testnet in adminConfig. Also prettier-reformats three pre-existing files.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR consolidates the prior extras work into main ahead of public release, bringing the vault contract and the React dApp into a single unified codebase with KYC-gated flows and a multisig-driven admin console.
Changes:
- Vault contract: adds KYC allowlist gating (deposit/mint/transfer), instance TTL extension, and an on-chain LP holder counter.
- App: adds
/adminmultisig signature-collection flow for allowlist management; improves dashboard read resiliency and adds LP count display; pins the app to a single vault contract id. - Tooling/deploy: adds Vitest for pure-logic modules,
@/path alias, and Vercel SPA rewrites.
Reviewed changes
Copilot reviewed 36 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vercel.json | Adds SPA rewrites for deep links on Vercel. |
| README.md | Updates repo documentation to reflect KYC + multisig admin and new app structure. |
| package-lock.json | Adds Vitest and lockfile updates for new dev dependencies. |
| contracts/vault/src/test.rs | Adds contract tests for allowlist gating and LP count transitions. |
| contracts/vault/src/lib.rs | Implements allowlist, owner/admin wiring, instance TTL extension, and LP counting. |
| contracts/vault/Cargo.toml | Adds stellar-access and stellar-macros dependencies for ownership gating. |
| Cargo.toml | Adds workspace dependencies for stellar-access and stellar-macros. |
| Cargo.lock | Locks new Rust crates introduced by the vault changes. |
| app/vitest.config.ts | Configures Vitest to run only src/lib pure-logic tests. |
| app/vite.config.ts | Adds @ path alias to src/. |
| app/tsconfig.node.json | Includes vitest.config.ts in the node tsconfig. |
| app/tsconfig.app.json | Adds TS path mapping for @/*. |
| app/src/providers/WalletProvider.tsx | Fixes stale poller closure; pins connected address; keeps session on transient errors. |
| app/src/providers/NotificationProvider.tsx | Minor type formatting change (no behavioral change). |
| app/src/pages/AdminPage.tsx | Adds /admin UI for composing/signing/submitting allowlist multisig transactions. |
| app/src/lib/vaultClient.ts | Introduces a single pinned vault client used across the app. |
| app/src/lib/txSignatures.ts | Adds signature merge + dedupe logic for multisig signing rounds. |
| app/src/lib/txSignatures.test.ts | Unit tests for signature merge behavior. |
| app/src/lib/threshold.ts | Adds signature-weight scoring by verifying signatures (not hint matching). |
| app/src/lib/threshold.test.ts | Unit tests for threshold/weight computation behavior. |
| app/src/lib/format.ts | Formatting refactor (no behavioral change intended). |
| app/src/lib/errors.ts | Improves Horizon submission error messages by surfacing result_codes. |
| app/src/lib/adminTx.ts | Implements RPC build/simulate/assemble/submit+poll for admin tx flow. |
| app/src/lib/adminLink.ts | Implements #tx= base64url hash-fragment transport for admin links. |
| app/src/lib/adminLink.test.ts | Unit tests for admin-link encoding/decoding and validation. |
| app/src/lib/adminConfig.ts | Pins testnet vault contract id and gov multisig account id. |
| app/src/hooks/useVault.ts | Adds LP count + resilient reads; adds useIsAllowed query for KYC gating. |
| app/src/hooks/useIsAdmin.ts | Adds admin gating based on gov multisig signers. |
| app/src/hooks/useAccountSigners.ts | Loads gov signers/thresholds from Horizon for the admin flow. |
| app/src/components/vault/VaultDashboard.tsx | Adds LP count stat, KYC-gated deposit UI, network mismatch banner, explorer link, and resilient loading behavior. |
| app/src/components/AnimatedNumber.tsx | Minor formatting refactor (no behavioral change intended). |
| app/src/App.tsx | Adds /admin route and nav entry gated by useIsAdmin. |
| app/package.json | Adds vitest run test script and dependency. |
| app/CLAUDE.md | Updates frontend dev/testing guidance; documents new admin + lib modules. |
| app-lib/env.ts | Tightens env validation (rpc/horizon URLs must be valid URLs). |
| app-lib/clients/vault/src/index.ts | Updates generated TS client for new contract id + new allowlist/LP-count methods and constructor args. |
| app-lib/clients/vault/README.md | Updates generated client docs for the new contract id. |
| app-lib/clients/vault/package-lock.json | Removes generated client lockfile (cleanup). |
| AGENTS.md | Updates repo-wide agent/developer guidance to reflect KYC + multisig + LP count. |
| .gitignore | Ignores local scaffold cache and Soroban test snapshot artifacts. |
Files not reviewed (1)
- app-lib/clients/vault/package-lock.json: Generated file
Comments suppressed due to low confidence (2)
contracts/vault/src/lib.rs:135
transfer_fromis a mutating entrypoint but does not callextend_instance_ttl, despite the contract comment stating every mutating entrypoint should extend instance TTL. Since it also updates the LP counter, it should extend TTL before writing instance storage.
app/src/components/vault/VaultDashboard.tsx:266- The withdraw tab UI is share-denominated (
bvUSDCunit,availableuses share balance), but the code builds awithdraw({ assets })tx.withdrawexpects an underlying-asset amount, while this flow is clearly redeem-by-shares (andestimatealready converts shares→USDC). This mismatch can produce incorrect behavior if share price != 1.
const tx =
tab === "deposit"
? await vault.deposit(args as never, { publicKey: address })
: await vault.withdraw(args as never, { publicKey: address })
const sent = await tx.signAndSend({ signTransaction })
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+119
to
+123
| fn transfer(e: &Env, from: Address, to: MuxedAddress, amount: i128) { | ||
| let to_address = to.address(); | ||
| if !AllowList::allowed(e, &from) || !AllowList::allowed(e, &to_address) { | ||
| panic_with_error!(e, VaultError::NotAllowed); | ||
| } |
Comment on lines
+52
to
+59
| let count = lp_count_read(e); | ||
| let updated = if before == 0 && after > 0 { | ||
| count + 1 | ||
| } else if before > 0 && after == 0 { | ||
| count - 1 | ||
| } else { | ||
| return; | ||
| }; |
This branch was successfully deployed
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
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.
What
Consolidates the
extrassandbox branch intomainso the repo is unified — no more split between a clean base vault and a separate feature branch — ahead of making it public.Changes
Contract (
contracts/vault)App
/adminmultisig page: collect signatures forallow/disallowand submit only once the threshold is met (link-passing between signers)@/path aliasNotes
mainis clean (no conflicts). The wallet-pin fix already onmain(fix(app): pin connected wallet account and stop poll flicker #10) is present onextrasas well.maindeploy serves the full app. Config is currently hardcoded to testnet, so no new env vars are required for it to build; migrating config to env vars is a follow-up for the public release.