Skip to content

fix(remappings): pin ds-test to direct forge-std path so make deploy reproduces canonical addresses - #171

Merged
amiecorso merged 1 commit into
coinbase:mainfrom
amiecorso:fix/remappings-ds-test-path
Jul 17, 2026
Merged

fix(remappings): pin ds-test to direct forge-std path so make deploy reproduces canonical addresses#171
amiecorso merged 1 commit into
coinbase:mainfrom
amiecorso:fix/remappings-ds-test-path

Conversation

@amiecorso

@amiecorso amiecorso commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Problem

make deploy fails at assert(implementation == EXPECTED_IMPLEMENTATION) in script/DeployFactory.s.sol, deploying the implementation to a non-canonical address (0xcB57…) instead of 0x00000110dCdEdC9581cb5eCB8467282f2926534d. The failure surfaces only as a bare panic: assertion failed (0x01) deep in the deploy script, which is hard to diagnose.

Root cause

The committed remappings.txt resolves ds-test through a nested copy:

ds-test/=lib/p256-verifier/lib/forge-std/lib/ds-test/src/

Solidity records the full remappings list in contract metadata (not just the remappings a given contract imports), and with the default bytecode_hash = "ipfs" that metadata hash is appended to the creation bytecode. Because CREATE2 hashes over the full init code, the stray ds-test path changes the deployed address, so the deterministic-deploy asserts fail. Both ds-test paths exist on disk (lib/forge-std/lib/ds-test and the nested lib/p256-verifier/lib/forge-std/lib/ds-test), so foundry's auto-resolution is ambiguous and the "wrong" one got committed.

Fix

Pin ds-test to the direct forge-std path, matching what the canonical v1.1 deployment used:

ds-test/=lib/forge-std/lib/ds-test/src/

Verified locally (forge clean && forge build, deploy profile) that both addresses reproduce exactly:

  • implementation → 0x00000110dCdEdC9581cb5eCB8467282f2926534d
  • factory → 0xBA5ED110eFDBa3D005bfC882d75358ACBbB85842

Note: forge clean is required after pulling this change, since stale artifacts otherwise mask it.

CI status — the red checks are pre-existing and should be left alone

This PR changes a single line in remappings.txt (+1/−1) and touches no .sol, no submodule, and no .gitmodules. The failing checks are unrelated repo-health/CI issues that predate this PR:

  • verify (...) (Certora, ×3) — the formal-verification jobs; expected/known.
  • Run Forge Tests and Checks — the failing step is forge fmt --check flagging trailing whitespace in an unrelated gas-profiling test file. Compilation itself succeeds. Not touched by this PR.
  • Run Coverage Reporting — fails at forge install with fatal: unable to read tree (5ea5d9f…) for solady. That revision is not referenced anywhere in the repo (.gitmodules pins solady at the fetchable c4c96607, v0.0.175); it's a phantom from CI submodule caching / the nightly forge build, not from repo content.

Why we are intentionally not "fixing" CI by bumping solady: solady is compiled directly into CoinbaseSmartWallet (Receiver, LibString, SignatureCheckerLib, UUPSUpgradeable are inherited and appear in the wallet's metadata sources). Bumping it would change the wallet's init code and metadata hash → change the deterministic CREATE2 address → break redeployment of the canonical 0x00000110… / 0xBA5ED1… addresses already live across 248 chains. This repo is effectively a frozen artifact for CBSW v1.0.0 / v1.1.0 whose purpose is reproducing those exact addresses, so the compiled dependency tree must stay byte-frozen. The correct scope for this PR is the one-line remappings fix only.

The committed remappings.txt resolves ds-test through a nested copy
(lib/p256-verifier/lib/forge-std/lib/ds-test/src/). Solidity records the full
remappings list in contract metadata, and with bytecodeHash=ipfs that metadata
hash is appended to the creation bytecode. Since CREATE2 hashes the full init
code, the stray path shifts the deterministic deployment address and 'make
deploy' fails at the EXPECTED_IMPLEMENTATION assert in DeployFactory.s.sol.

Pin ds-test to lib/forge-std/lib/ds-test/src/ to match the canonical v1.1
deployment. Verified locally (forge clean + forge build, deploy profile) that
both the implementation (0x00000110...) and factory (0xBA5ED1...) addresses
reproduce exactly.

Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
@cb-heimdall

cb-heimdall commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@amiecorso
amiecorso merged commit d0c113e into coinbase:main Jul 17, 2026
3 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants