Skip to content

fix: redeploy testnet vault with migrate_adapter (#514) - #544

Merged
collinsezedike merged 11 commits into
drydocs:mainfrom
blockchain-maxis:feat/redeploy-testnet-vault-migrate-adapter
Aug 22, 2026
Merged

fix: redeploy testnet vault with migrate_adapter (#514)#544
collinsezedike merged 11 commits into
drydocs:mainfrom
blockchain-maxis:feat/redeploy-testnet-vault-migrate-adapter

Conversation

@blockchain-maxis

@blockchain-maxis blockchain-maxis commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

closes #514

⚠️ Before merging this PR

The previous testnet vault (CBQYEHWIRJWIPWCJFQZAOP3VAZHRWFGAUS5GZHWFDDYKMFHJ5S3YS2Q5) held get_total_assets() = 200000000000 (20,000 USDC) in outstanding deposits when this was checked. This is a breaking cutover — the old vault has no on-chain redemption path once the app stops pointing at it (see the #514 discussion), only a manual withdraw() against the now-unlisted old address.

Please hold this PR open for a pre-cutover announcement window so anyone with a position on the old vault can withdraw before merge. Once merged, KNOWN_POOLS/CONTRACT_ADDRESSES point at the new vault and the old one is no longer reachable through the UI (it keeps running and withdraw() still works on it directly — see apps/docs/operations/testnet-deployment.md's new "Vault migration history" section — but nothing in the app links to it anymore).

Summary

packages/contracts/vault/src/lib.rs has had migrate_adapter since #464/#507 merged into main, but the live testnet vault was never redeployed since, so it didn't have it — confirmed both by stellar contract invoke ... -- --help (function missing) and by .github/workflows/verify-contract-addresses.yml's "Verify On-Chain Bytecode" job (hash mismatch). This PR redeploys a fresh vault + BlendAdapter + mUSDC and repoints every reference at it.

What changed, file by file (one commit each)

  1. scripts/deploy-testnet.sh — fixed a real bug I hit while running this: MeridianVault::initialize() calls admin.require_auth(), so the script's vault-init step fails with Missing signing key for account $ADMIN_ADDRESS whenever ADMIN is a genuinely separate address from DEPLOYER — which is the script's own recommended usage for anything beyond a throwaway test. The script never has (and shouldn't have) ADMIN's secret key, so it now builds up to that point and prints the initialize() command for whoever holds the ADMIN key to run themselves, using ADMIN as the invoking source account (so its own signature satisfies require_auth()). This mirrors the existing "print, don't auto-run" pattern redeploy-blend-adapter.sh already uses for set_adapter. The ADMIN-defaults-to-DEPLOYER throwaway path is untouched and still fully automatic.
  2. packages/shared/src/constants.tsCONTRACT_ADDRESSES.testnet.vault, .musdc, and MUSDC_ISSUER.testnet now point at the redeployed contracts. Also corrected the MUSDC_ISSUER comment, which said the issuer was ADMIN's address — it's actually DEPLOYER's, per how the script mints the asset (MUSDC:$DEPLOYER_ADDRESS).
  3. packages/stellar-sdk-helpers/src/known-pools.tsKNOWN_POOLS.testnet["meridian-usdc"].contractId updated to match, so scripts/verify-contract-addresses.ts's internal-consistency check (the two config sources must agree) still passes.
  4. packages/stellar-sdk-helpers/src/tx.test.ts — this test hardcoded a duplicate literal of the old MUSDC_ISSUER.testnet value instead of importing it, so redeploying broke allows a changeTrust to the known mUSDC issuer without me touching this file directly. Fixed by importing MUSDC_ISSUER from @meridian/shared instead of duplicating it, so the next redeployment doesn't require hunting this down again.
  5. apps/web/src/__tests__/hooks/useTrustlines.test.ts — same drift, same fix.
  6. apps/web/src/__tests__/hooks/useVaultActions.test.ts — same drift, same fix.
  7. apps/web/e2e/withdraw.spec.ts — this test asserted the withdraw simulation fails with exactly InsufficientShares (#7), which only held because the previous vault already had other depositors (total_shares > 0) and this test's account specifically held none. MeridianVault::withdraw() checks total_shares <= 0 (NoSharesOutstanding, #6) before the per-caller check, so a freshly redeployed vault with zero deposits from anyone now returns #6 for the same request — verified directly against the live API, not guessed. Both are the correct "nothing to withdraw" rejection this test actually cares about, so it now matches either via regex instead of pinning to whichever the vault's deposit history happens to produce.
  8. apps/docs/operations/testnet-deployment.md — added the "Vault migration history" section (the durable record the maintainer asked for in the issue thread) documenting the old vault's address/mUSDC/issuer/admin, that it's still live and withdraw()-able, its balance at cutover, and the new vault's addresses plus how they were verified. Also corrected "The DEPLOYER / ADMIN split" section, which previously implied ADMIN never needs to sign anything — it does, per the bug fixed in commit 1 — updated the "Standing up a fresh environment" walkthrough to match, and added a new note on reproducible builds (see below).

New testnet deployment

Contract Address
Vault CBOE7JPROCMUKQ4NJWPKCLBBQGHLTGV4X3463DHK4D7KX6KWXGZETAJL
Blend adapter CDFIDKNA2ZTB37I7RN32WH7VU5AP2PAOXLGFWMTW6T2RSUM23AJIV2YM
mUSDC CCSYXC4SDCPTGENHM6CSQY4HMSZOPOY5TJW4QYYLE5RDBUBJX4N7ZHV5
mUSDC issuer (DEPLOYER) GBLYQ5EHXMMULOA7KA4KK2S5Q5GTTWYFVSC3FKLXRLH34EJX35BIAL35
Admin GB74ZDVMBYMPKWBBVJ7TAN2QK2EAKQQ5OZO6ETYAMPN5VQVNLZSQUYHH

ADMIN is a fresh, separate key generated specifically for this deployment (not the deploying key), per the docs' own guidance. Its secret is currently held by me as this PR's author; it can be rotated to a different durable key at any time via set_admin (no redeploy required) if maintainers want different long-term custody. I did not reuse the previous vault's admin address (GDZX7DOZMVEZJSWPDIZCTSCAKW4LBB3UGNWYAG5YTCBL4JPMUPAWWEUD) since I don't hold its secret key, and initialize()'s require_auth() makes that a hard requirement, not a formality.

An important gotcha I hit: reproducible builds

My first deploy attempt on this PR passed every check I could run locally, but failed CI's "Verify On-Chain Bytecode" job with a hash mismatch. Root cause: stellar contract build's WASM output is not guaranteed byte-identical across stellar-cli versions, even from identical source — my local stellar-cli (25.2.0) was two major versions behind what CI's unpinned cargo install --locked stellar-cli resolves to at run time (27.1.0), which applies a different default optimization pass and pulls different soroban-sdk transitive versions. Same source, different bytes.

Rather than fight my local toolchain into matching CI's exactly, I built the vault from this branch's source using GitHub Actions itself (a throwaway workflow_dispatch job on my fork that builds and uploads the .wasm as an artifact, since deleted), downloaded that exact artifact, and deployed that — guaranteeing the on-chain bytecode is byte-for-byte what CI's own rebuild will produce, rather than hoping a local build happens to match. Confirmed: stellar contract fetch on the deployed vault hashes to 1fd957b09c95326583aa708e59fd91647f05157b8776656bae72c5de8ceaeb4b, identical to what CI's job computes from source. Documented this as a general note in testnet-deployment.md so the next person deploying doesn't lose time to the same mismatch.

Verification performed

  • On-chain bytecode of the deployed vault matches a from-source rebuild done on GitHub Actions, byte-for-byte (see above) — this is exactly what .github/workflows/verify-contract-addresses.yml's required "Verify On-Chain Bytecode" job independently checks.
  • stellar contract invoke ... -- --help on the new vault lists migrate_adapter alongside all the other expected functions.
  • vault.get_adapter() resolves to the new BlendAdapter; that adapter's get_pool() / get_protocol() resolve to the Blend testnet pool / "blend" — the same discovery chain the frontend uses for live APY.
  • vault.get_total_assets() on the new vault returns 0 (fresh, no deposits yet), as expected.
  • cargo test --release in packages/contracts: 43 passed, 0 failed (vault crate), plus clean adapter crate builds.
  • pnpm --filter @meridian/shared test: 98 passed.
  • pnpm --filter @meridian/stellar-sdk-helpers test: 189 passed (includes the test fixed in commit 4).
  • pnpm --filter web test: 90 passed (includes the two files fixed in commits 5–6).
  • Hit the real, running local API (POST /api/v1/tx/withdraw against the new vault) directly to confirm the exact error code before writing the withdraw.spec.ts fix in commit 7, rather than guessing at it.
  • pnpm --filter @meridian/shared typecheck and ...stellar-sdk-helpers typecheck: clean.
  • pnpm --filter web lint: clean.
  • pnpm format:check: clean (ran prettier --write on the touched Markdown/TS files).

Not run / pre-existing, unrelated to this change: pnpm install --frozen-lockfile currently fails with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH because the locally available pnpm (10.22.0) doesn't match the repo's pinned packageManager (pnpm@9.0.0). Worked around locally with --no-frozen-lockfile for testing only; pnpm-lock.yaml is unchanged in this PR (verified with git status before every commit). Flagging in case it's worth a separate look, but intentionally not bundled into this fix.

Why I didn't also touch the frontend or other tooling

The issue asked to update "every hardcoded reference (frontend, docs, any other tooling) together in one pass." I grepped the whole repo for the old vault address and the old mUSDC address before starting: the only hits were the two config files this PR updates (constants.ts, known-pools.ts), the two test files fixed in commits 5–6 (their own hardcoded duplicates, now sourced from @meridian/shared instead), plus the doc file's own historical mentions (intentionally left as history in commit 8, since apps/docs/operations/testnet-deployment.md is the durable record). The adapter address was already not hardcoded anywhere (the frontend discovers it live via vault.get_adapter()), and the frontend/API otherwise read vault/mUSDC addresses through CONTRACT_ADDRESSES/KNOWN_POOLS, not their own copies.

@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

@blockchain-maxis is attempting to deploy a commit to the Collins' projects Team on Vercel.

A member of the Team first needs to authorize it.

@blockchain-maxis blockchain-maxis changed the title Redeploy testnet vault with migrate_adapter (#514) fix: redeploy testnet vault with migrate_adapter (#514) Aug 20, 2026

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two more things not anchorable to a specific diff line:

  • E2E Tests fails on the withdraw spec: it submits a withdraw against this new vault expecting a specific simulation-failure message that never appears, meaning the new vault's behavior or initialization state doesn't match what this PR assumes.
  • One commit header (fix(scripts): sign vault initialize() with ADMIN when separate from DEPLOYER) is 76 characters, over the 72-char limit, blocking Commit Messages.

Comment thread packages/shared/src/constants.ts Outdated
// added it. See apps/docs/operations/testnet-deployment.md's "Vault
// migration history" for the old address, why it's stale, and the
// pre-cutover withdrawal window for anyone still holding a position there.
vault: "CC2BI7LDKXFTSODQ7CAILWB6APFHXDJLFN5PRX6G6VR5FVWDC4C2IYBY",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The Verify On-Chain Bytecode check fails for this address: the on-chain bytecode at this contract doesn't match the vault WASM built from this repo's source. That directly contradicts the PR description's claim that pnpm verify:contracts passes clean. Given this is redeploying the live testnet vault, this needs to be resolved before merge, not just noted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Root cause: my local stellar-cli (25.2.0) was two major versions behind CI's unpinned cargo install --locked stellar-cli (27.1.0), which produces different WASM bytecode from identical source. Rather than fight my local toolchain into matching, I built the vault on GitHub Actions itself (a throwaway workflow_dispatch job on my fork, since deleted) and deployed that exact artifact, so the on-chain bytecode is byte for byte what CI's own rebuild produces. Confirmed with stellar contract fetch against the deployed vault: hash 1fd957b09c95326583aa708e59fd91647f05157b8776656bae72c5de8ceaeb4b, identical to what the Verify On-Chain Bytecode job computes. That check now passes on the latest commit. Documented the general gotcha in apps/docs/operations/testnet-deployment.md so it does not bite the next person. New vault address is CBOE7JPROCMUKQ4NJWPKCLBBQGHLTGV4X3463DHK4D7KX6KWXGZETAJL, now in this file.

@blockchain-maxis
blockchain-maxis force-pushed the feat/redeploy-testnet-vault-migrate-adapter branch from 6e010f2 to 6a4d0f9 Compare August 21, 2026 06:43
MeridianVault::initialize() calls admin.require_auth(), so whenever ADMIN
is set to a genuinely separate address from DEPLOYER (the script's own
recommended usage for anything beyond a throwaway run), submitting the
call with only DEPLOYER's signature fails with 'Missing signing key for
account $ADMIN_ADDRESS'. Discovered while redeploying the testnet vault
for drydocs#514.

The script never has access to ADMIN's secret key by design (that's the
whole point of the DEPLOYER/ADMIN split), so it can't sign this on its
own. When ADMIN != DEPLOYER, build the invoke up to that point and print
the initialize() command for whoever holds the ADMIN key to run
themselves, using ADMIN as the invoking source account so its own
signature satisfies admin.require_auth(). This mirrors the existing
'print, don't auto-run' pattern redeploy-blend-adapter.sh already uses
for set_adapter, for the same reason.

The DEPLOYER-defaults-to-ADMIN throwaway path is untouched and still
fully automated, since DEPLOYER's signature already covers it in that
case.
The live testnet vault predated migrate_adapter (added in drydocs#464/drydocs#507) and
was never redeployed since, which .github/workflows/verify-contract-addresses.yml's
bytecode check caught. Redeployed via scripts/deploy-testnet.sh with a
fresh, separate ADMIN key (per the DEPLOYER/ADMIN split guidance) rather
than defaulting ADMIN to the deploying key.

- CONTRACT_ADDRESSES.testnet.vault -> the new vault
  (CBOE7JPROCMUKQ4NJWPKCLBBQGHLTGV4X3463DHK4D7KX6KWXGZETAJL), which
  exports migrate_adapter, resolves get_adapter() -> the new BlendAdapter
  -> get_pool()/get_protocol(), and whose on-chain bytecode was verified
  byte-for-byte against a from-source rebuild done on GitHub Actions
  (not a local machine -- stellar-cli builds are not guaranteed
  reproducible across toolchain versions, see the new note in
  apps/docs/operations/testnet-deployment.md).
- CONTRACT_ADDRESSES.testnet.musdc and MUSDC_ISSUER.testnet -> the new
  mUSDC share token minted for this deployment, and its issuer (the new
  DEPLOYER address, per how deploy-testnet.sh mints it).
- Corrected the MUSDC_ISSUER comment, which described the issuer as
  ADMIN's address; it's actually DEPLOYER's, per the script.

The previous vault (CBQYEHWIRJWIPWCJFQZAOP3VAZHRWFGAUS5GZHWFDDYKMFHJ5S3YS2Q5)
is left running as-is, not deleted or disabled -- see
apps/docs/operations/testnet-deployment.md's new migration-history entry
for its full details, its outstanding balance at cutover time, and the
pre-merge withdrawal window for anyone still holding a position there.
KNOWN_POOLS.testnet["meridian-usdc"].contractId -> the vault redeployed
for drydocs#514, matching CONTRACT_ADDRESSES.testnet.vault. Keeps the two in
agreement, which scripts/verify-contract-addresses.ts's internal-
consistency check requires.
tx.test.ts hardcoded a duplicate literal of MUSDC_ISSUER.testnet instead
of importing it, so redeploying the vault for drydocs#514 (which changes that
issuer) broke "allows a changeTrust to the known mUSDC issuer" without
touching this file at all. Import MUSDC_ISSUER from @meridian/shared and
derive the fixture from it in both places it was duplicated, so the next
vault redeployment doesn't require hunting down stray copies here.
Same drift as tx.test.ts in packages/stellar-sdk-helpers: this file
hardcoded a duplicate of MUSDC_ISSUER.testnet (and USDC_ISSUER.testnet)
instead of importing them, so the drydocs#514 vault redeployment broke it too.
Import both from @meridian/shared instead.
Same drift as the two previous commits: this file hardcoded its own
duplicate of MUSDC_ISSUER.testnet / USDC_ISSUER.testnet instead of
importing them, so the drydocs#514 vault redeployment broke it too. Import both
from @meridian/shared instead.
withdraw.spec.ts asserted the withdraw simulation fails with exactly
InsufficientShares (drydocs#7), which held only because the previous testnet
vault already had other depositors (so total_shares > 0) and this test's
account specifically held none. MeridianVault::withdraw() checks
total_shares <= 0 (NoSharesOutstanding, drydocs#6) before the per-caller check,
so the drydocs#514 redeployment -- a vault with zero deposits from anyone --
now returns drydocs#6 for the same request, which the old exact-match assertion
missed.

Both are the correct "nothing to withdraw" rejection this test cares
about (verifying the request is rejected before signing, not which
specific contract error produces the rejection), so match either via
regex instead of pinning to whichever the live vault's deposit history
happens to produce.
…ing gap

- New "Vault migration history" section: a durable, dated record of the
  drydocs#514 cutover. Documents the superseded vault's address/mUSDC/issuer/
  admin, that it's still live and withdraw()-able (not deleted, Soroban
  has no such op), its outstanding balance at cutover time with a
  pre-merge withdrawal window, that there's no automatic migration of old
  positions, and the new vault's addresses plus how they were verified
  against drydocs#514's acceptance criteria, including a byte-for-byte on-chain
  bytecode check against a GitHub Actions rebuild.
- New note on reproducible builds: stellar-cli's WASM output isn't
  guaranteed identical across toolchain versions, so a locally-built
  contract can genuinely mismatch what CI's verify-contract-addresses
  workflow rebuilds and compares against, independent of whether the
  source is correct. Explains how to check for and avoid this.
- "The DEPLOYER / ADMIN split": corrected to say initialize() needs
  ADMIN's own signature when it's a separate address (previously implied
  it never needed one), and describes the script's new build-and-print
  behavior for that case instead of assuming full automation.
- "Standing up a fresh environment": step 2 now reflects that vault
  initialization is automatic only when ADMIN defaults to DEPLOYER, and
  printed for manual completion otherwise.

Formatted with prettier per the repo's format:check.
@blockchain-maxis
blockchain-maxis force-pushed the feat/redeploy-testnet-vault-migrate-adapter branch from 6a4d0f9 to 22927a9 Compare August 21, 2026 06:47
@blockchain-maxis

Copy link
Copy Markdown
Contributor Author

@collinsezedike Both fixed as of the latest commit:

Also fixed the bytecode mismatch flagged in the other comment (root cause was a stellar-cli version gap between my machine and CI). All checks are green now except Vercel, which needs a maintainer authorization on your side, not something on my end.

Comment thread scripts/deploy-testnet.sh Outdated
echo " --id $VAULT_ID -- initialize \\"
echo " --admin $ADMIN_ADDRESS --usdc $USDC_ID --musdc $MUSDC_ID --adapter $BLEND_ADAPTER_ID"
echo ""
echo "The vault is deployed but NOT YET INITIALIZED until that runs."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

initialize() has no restriction on who can call it, it only checks the vault isn't already initialized and that admin.require_auth() passes for whatever address is passed as the parameter. The old script had DEPLOYER call initialize() immediately in the same run, so the uninitialized window was sub-second. This branch instead deploys the vault, prints the initialize() call, and exits, leaving a live, publicly-callable, uninitialized vault on testnet indefinitely.

Anyone watching testnet deployments can call initialize() first with themselves as admin, using the same usdc/musdc/adapter values from this script's own output. The legitimate ADMIN key holder's later attempt then fails with AlreadyInitialized, and the attacker, now the vault's admin, can call set_adapter (unrestricted while total_shares == 0) to redirect the first real depositors' USDC through an adapter they control.

This is the same bug class as #505 (front-runnable initialize()), just on the vault instead of the adapters, and you already have the atomic-constructor fix agreed for that one. Worth applying the same approach here rather than a print-and-wait step.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, and this is my bug. The old script had DEPLOYER deploy and initialize back to back, so the window was one ledger. My change turned that into an open-ended one, and initialize() genuinely doesn't care who calls it as long as admin.require_auth() passes for whatever address they pass in. Someone can claim the vault with themselves as admin, and since set_adapter is only gated on total_shares > 0, a fresh vault with no deposits is wide open to being repointed at an adapter they control.

On the vault currently in this PR: get_admin() on CBOE7JPROCMUKQ4NJWPKCLBBQGHLTGV4X3463DHK4D7KX6KWXGZETAJL returns GB74ZDVMBYMPKWBBVJ7TAN2QK2EAKQQ5OZO6ETYAMPN5VQVNLZSQUYHH, the intended ADMIN, and get_total_shares() is 0, so nobody claimed it during the window I left open. That's luck though, not the design working.

The fix is the same shape as the one we agreed on for #505: move the init body into __constructor(admin, usdc, musdc, adapter) so it runs inside the CreateContract host op in the deploying transaction. soroban-sdk is pinned at 22.0.0 here, which supports it, so the script becomes a single stellar contract deploy ... -- --admin ... --usdc ... --musdc ... --adapter ... invoked with ADMIN as the source account. ADMIN's signature satisfies require_auth() in the same transaction, which also solves the original problem this commit was trying to solve, with no separate call left to race or to print. I'd keep initialize() on the contract so the already-deployed vault's ABI doesn't change, but it becomes unreachable on constructor-deployed vaults since ADMIN is set before any external call can land.

One sequencing question before I push that, because it isn't free here. Adding a constructor changes the vault WASM, so CBOE7JPRO... stops matching a from-source rebuild and Verify On-Chain Bytecode goes red, which means redeploying vault + adapter + mUSDC again and another pass over the addresses in this PR. #505 is assigned to me and lands the identical constructor change on BlendAdapter and DefindexAdapter. Two options:

  1. Do it here now, then redeploy a third time when [Bug] Adapter initialize() is unauthenticated, front-runnable #505 lands the adapter constructors.
  2. Do the constructor work in [Bug] Adapter initialize() is unauthenticated, front-runnable #505 covering the vault and both adapters together, then rebase this PR on it and deploy once with all three fixed.

I'd lean towards 2, since this PR can't merge until #513 and the withdrawal window clear anyway, and it means one cutover announcement instead of two. Happy to do 1 if you'd rather not couple the two PRs.

Either way, I'll change this commit now so the branch doesn't sit in a worse state than main while we decide: the script takes an optional ADMIN signing alias and, when it has one, runs initialize() as the immediate next call the way it always did, keeping the window at one ledger. Print-and-wait stays only as the fallback for when the key genuinely isn't on that machine, with a loud warning that the vault is claimable until it runs. That's defense in depth under the constructor fix, not a replacement for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interim change is in, 4bc82ba. The script now takes an optional ADMIN_KEY (secret key or stellar keys alias) and, when it has one, signs and submits initialize() itself in the same run, in exactly the position it always ran, so the window is one ledger again. ADMIN_KEY is resolved and checked against ADMIN before the build, so a mismatch exits without spending a deploy. With no ADMIN_KEY it still prints the call, but now says outright that the vault is claimable until it lands and gives the get_admin check to run afterwards. Docs updated to match.

Still just defence in depth. The __constructor change is the actual fix, and I'm holding it for your call on sequencing in the comment above.

@@ -242,8 +242,10 @@ const TESTNET: StellarNetwork = {

const USDC_ISSUER_TESTNET =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR's own stated fix is only half-applied here: MUSDC_ISSUER_TESTNET below is converted to source from @meridian/shared, but USDC_ISSUER_TESTNET stays hardcoded right next to it, the exact pattern this PR exists to eliminate. USDC_ISSUER is already exported from @meridian/shared and used the same way elsewhere in this file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair, that's half a fix. I only touched MUSDC_ISSUER_TESTNET because that's the one the redeployment broke, but leaving the literal for USDC_ISSUER_TESTNET sitting on the line above it is exactly the drift this PR is supposed to stop. Same duplicate exists again in the buildAddTrustlineTx via API block lower down at line 435.

Fixing both to import USDC_ISSUER from @meridian/shared and read USDC_ISSUER.testnet, same as the mUSDC one.

One I'm leaving as a literal on purpose: UNKNOWN_ISSUER at line 442 happens to hold the same value as USDC_ISSUER.mainnet, but its job in the test is to be a well-formed address that isn't on the testnet allowlist. Sourcing it from the mainnet constant would make the test read as though it cares about mainnet USDC, and it would silently stop testing anything if mainnet ever got added to the allowlist. Say the word if you'd rather it come from the constant anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 7646eea. USDC_ISSUER is imported from @meridian/shared and both fixture blocks read USDC_ISSUER.testnet. UNKNOWN_ISSUER left as a literal for the reason above, happy to change it if you disagree.

@collinsezedike

Copy link
Copy Markdown
Collaborator

@blockchain-maxis this PR can't merge yet regardless of review outcome. Two things outside the code itself have to resolve first:

If you haven't already, might be worth getting started on #505 in the meantime.

@blockchain-maxis

Copy link
Copy Markdown
Contributor Author

@collinsezedike Understood, no objection to holding. I'd already flagged the withdrawal window at the top of the PR description, so it stays open until the announcement goes out and its window passes, and until #513 is resolved. Ping me when you want the cutover timed and I'll rerun the balance check on the old vault right before merge so the number in the announcement is current rather than the one I captured a few days ago.

On #505: yes, I'm on it, and it turns out to overlap with your initialize() comment on scripts/deploy-testnet.sh here. I've replied there with the details, but the short version is that the vault needs the same __constructor treatment as the adapters, and doing both in #505 would let this PR redeploy once with all three contracts fixed instead of cutting over twice. Left the sequencing call to you in that thread.

tx.test.ts kept a hardcoded copy of USDC_ISSUER.testnet in both fixture
blocks, the same drift pattern the mUSDC issuer fixture was just fixed
for. Import USDC_ISSUER from @meridian/shared and read .testnet from it
instead, so neither issuer fixture has to be hunted down by hand on the
next redeployment.

UNKNOWN_ISSUER is deliberately left as a literal: its job is to be a
well-formed address that is absent from the testnet allowlist, not to
track a real issuer constant.
The previous commit made deploy-testnet.sh print initialize() and exit
whenever ADMIN was a separate address from DEPLOYER, since the script
cannot produce ADMIN's signature and admin.require_auth() demands it.
That left a live, uninitialized vault on testnet for as long as it took
the key holder to run the printed call.

initialize() is callable by any address and only checks that whatever
admin it is handed authorizes the call, so an uninitialized vault can be
claimed by whoever calls it first with their own address as admin. They
would then hold set_admin/set_paused/set_adapter, and set_adapter is
only gated on total_shares > 0, so a fresh vault can be repointed at an
adapter they control before the first real deposit lands.

Add an optional ADMIN_KEY (secret key or stellar keys alias) holding
ADMIN's signing key. When supplied, the script signs and submits
initialize() itself in the same run, exactly where it always ran, so
the window is one ledger again. ADMIN_KEY is resolved and checked
against ADMIN before the build, so a mismatch fails without spending a
deploy. Without it the script still prints the call, but now says
plainly that the vault is claimable until it runs and how to confirm
ownership with get_admin afterwards.

This is defence in depth, not the real fix. Moving initialize() into a
__constructor removes the window entirely, which is the approach agreed
for the adapters in drydocs#505 and applies to the vault the same way.
@collinsezedike

Copy link
Copy Markdown
Collaborator

@blockchain-maxis #513 is resolved (merged as #545), and the withdrawal announcement is posted: #546. Let's give users a few days to actually see it and withdraw before this merges, not just have it posted.

@blockchain-maxis

Copy link
Copy Markdown
Contributor Author

@collinsezedike Agreed on the window, no rush from my side. Thanks for getting #513 and the announcement out.

Baseline for it, taken just now against the old vault (CBQYEHWIRJWIPWCJFQZAOP3VAZHRWFGAUS5GZHWFDDYKMFHJ5S3YS2Q5):

get_total_assets() -> 200000000000
get_total_shares() -> 200000000000

Same 20,000 USDC I recorded when I opened this PR, so nobody has withdrawn yet as of about 40 minutes after #546 went up. I'll re-read both and post the number here once a day while the window runs, and again right before you merge, so the call is made on the current figure rather than one from a few days earlier.

One suggestion on #546: it doesn't name a date, and "before the migration lands" is hard for a reader to plan around, especially anyone who only checks Discussions occasionally. If you pick an actual cutover date and add it there, I'll hold this PR until then without needing to ask. Your discussion, so entirely your call, just flagging that the window only helps if people can tell how long it is.

Separately, I re-ran the reference check now that #545 is on main. Grepped current main for the old vault address and the old mUSDC address (CBC5G4HXTOOZHTBCJQACZB3NJ636JHA5NEBQX5Q265QZN6XEG4LVZ5SB): the only hits are the two config files this PR already updates. The new .github/workflows/keepers.yml doesn't reference either address, so there's nothing extra for this PR to pick up. I'll rebase onto main before merge so the final CI run is against current main rather than the tree from before #545.

Starting on #505 in the meantime. That also settles the constructor question I raised on the deploy-testnet.sh thread, since the same fix applies to the vault.

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@blockchain-maxis thank you for your contribution. The withdraw assertion fix and bytecode mismatch resolution are correct, and the daily balance checks on the old vault are appreciated. Merging now.

@collinsezedike
collinsezedike merged commit f0ce7f0 into drydocs:main Aug 22, 2026
9 of 10 checks passed
@collinsezedike

Copy link
Copy Markdown
Collaborator

@blockchain-maxis I appreciate the patience through the hold on #513 and the withdrawal window, and the daily balance checks along the way. Merged, thank you.

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.

[Bug] Live testnet vault predates migrate_adapter, needs redeployment

2 participants