Skip to content

DOC-44 Add smart contract gas optimization guide #845 FIXED - #922

Merged
anumukul merged 1 commit into
anumukul:mainfrom
veloura-dev:DOC-44-Add-smart-contract-gas-optimization-guide-#845-FIX
Sep 4, 2026
Merged

DOC-44 Add smart contract gas optimization guide #845 FIXED#922
anumukul merged 1 commit into
anumukul:mainfrom
veloura-dev:DOC-44-Add-smart-contract-gas-optimization-guide-#845-FIX

Conversation

@veloura-dev

@veloura-dev veloura-dev commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

What & Why

Fixes the address-formatting issue across the ETH ↔ Stellar bridge. ETH addresses were stored and queried verbatim in whatever casing the caller sent, so a GET /orders/history?address=… for the EIP-55 form of an address silently returned zero rows when the order had been announced with the lowercase form (and vice versa); whitespace-padded addresses were rejected instead of trimmed; malformed ETH token addresses in the SDK silently mapped to XLM (false asset match); and the frontend hardcoded a USDC contract address that is not the Sepolia USDC deployment.

This PR introduces one strict canonicalization layer and enforces it at every edge:

  • SDK (new addresses module): normalize + validate + compare ETH (EIP-55) and Stellar (G + 55 base32) addresses; strict canonicalization wired into the assets module so malformed ETH token addresses can no longer false-match XLM.
  • Coordinator: POST /api/orders/announce and GET /api/orders/history now validate + canonicalize (trim, EIP-55 check, Stellar format check), store lowercase canonical ETH addresses, and reject malformed input with explicit 400 errors (invalid_address with the expected checksum, order_validation_error with the precise reason) instead of storing/garbling it.
  • Relayer: canonicalization helpers + trim-tolerant validators.
  • Frontend: correct Sepolia USDC constant (0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238 — Ethereum L1 Sepolia, per Circle docs), canonical-case display in the token selector.
  • Ops/compat: coordinator's node:sqlite load is now lazy with a clear "requires Node >= 22.5" error; DB test suites skip cleanly on older runtimes (repo .nvmrc pins 24.2.0), so the suite is green on both Node 20 and Node ≥ 22.5.

Closes the address-formatting issue.

Touched surface

Tick every layer this PR changes:

  • frontend/ — React + Vite bridge UI
  • packages/sdk/ — shared TypeScript SDK
  • coordinator/ — order book + REST/WS service
  • resolver/ — community resolver runner
  • relayer/ — legacy v1 listener / watchdog (changes here need extra scrutiny)
  • contracts/ — Solidity v2 (HTLCEscrow, ResolverRegistry)
  • soroban/ — Stellar Soroban contracts (oversync-htlc, oversync-resolver-registry)
  • docs/ — documentation only
  • CI / config (.github/workflows/, docker, env.example)

Settlement & refund semantics

Critical for SCF / investor review. If any box is checked, the PR must also update docs/REVIEW_RESPONSE.md and link the updated section in the PR description.

  • Bridge settlement semantics changed (claim path, timelock ordering, preimage handling, hashlock type, asset routing)
  • Refund semantics changed (who can refund, who receives refunds, timelock values, refund-address pinning)
  • Settlement-critical invariant changed (e.g. non-custodial guarantee, no-admin-escape-hatch, permissionless refund)
  • None of the above — this PR cannot move, hold, or release user funds

Changed logic is API-edge validation + canonical storage/lookup of address strings only; claim/refund/timelock/hashlock state-machine code is untouched. docs/REVIEW_RESPONSE.md §10 documents this and was updated in this PR.

Tests run

Tick the matches your change and paste the outcome below. Commands mirror the matrix in CONTRIBUTING.md.

  • pnpm --filter @oversync/sdk build && pnpm --filter @oversync/sdk exec tsc --noEmit
  • pnpm --filter @oversync/sdk test
  • pnpm --filter @oversync/coordinator exec tsc --noEmit && pnpm --filter @oversync/coordinator test
  • pnpm --filter @oversync/resolver exec tsc --noEmit && pnpm --filter @oversync/resolver test
  • pnpm --filter @oversync/frontend exec tsc --noEmit && pnpm --filter @oversync/frontend test
  • pnpm --filter @oversync/contracts compile && pnpm --filter @oversync/contracts exec hardhat test test/v2/HTLCEscrow.test.ts test/v2/ResolverRegistry.test.ts
  • cd soroban && stellar contract build && cargo test --release — not run, soroban/ untouched
  • (cd contracts && forge test --match-path "test/foundry/*" -v) — not run, Solidity untouched
  • pnpm test:e2e (cross-chain differential harness) — run as the e2e package under pnpm test
  • node scripts/verify-addresses.mjs — required if addresses, configs, or env.example change
  • node scripts/check-evidence-links.mjs — advisory; required if docs links change

Free-form outcome:

Full matrix via `pnpm build` (6/6 packages) + `pnpm test`, run on BOTH runtimes:

Node v22.14.0 (>= 22.5, per repo .nvmrc 24.2.0):
  packages/sdk    13 files, 208/208 passed
  coordinator     14 files, 167/167 passed (incl. all 51 node:sqlite DB tests)
  resolver         5 files,  60/60 passed
  frontend        18 files, 133/133 passed
  relayer          2 files,  21/21 passed
  contracts       21 passing (hardhat)
  e2e              2 files,  22/22 passed
  => pnpm test exit 0

Node v20.20.2 (default CI-less dev runtime, no node:sqlite):
  identical, except coordinator: 10 files passed | 4 skipped, 116 passed + 51 skipped
  (the 51 = DB-persistence suites, skipped via describe.runIf(hasNativeSqlite())
   with a "requires Node >= 22.5" message — no silent failures, exit 0)

node scripts/verify-addresses.mjs       -> 21 verified, 0 failures
node scripts/check-evidence-links.mjs   -> 12 pre-existing broken links in docs,
                                           none introduced by this PR; every link
                                           added in REVIEW_RESPONSE.md §10 resolves

UI / evidence artefacts

Required if the PR changes the frontend, observability, dashboards, or any docs that claim status, metrics, or addresses. Otherwise write n/a.

  • Frontend visible change → screenshot or short clip attached (swap flow, refund dialog, history banner, wallet confirm)
    — no screenshot produced from this environment; the only visible change is canonical-case address rendering (frontend/src/components/TokenSelector.tsx) and the corrected Sepolia USDC constant (frontend/src/config/networks.ts).
  • Coordinator API changecurl snippet + JSON response sample pasted below

Captured live from this branch's coordinator (Node 22.14, real SQLite, port 3999):

$ curl -s -X POST localhost:3999/api/orders/announce -H 'content-type: application/json' \
    -d '{"direction":"eth_to_xlm","hashlock":"0x9f86…a08","srcChain":"ethereum",
         "srcAddress":"0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238","srcAsset":"USDC",
         "srcAmount":"1000000","srcSafetyDeposit":"0","dstChain":"stellar",
         "dstAddress":"GDQK…7AS6","dstAsset":"USDC:GDQK…7AS6","dstAmount":"1000000"}'
HTTP 201
{"id":"800910d2…","direction":"eth_to_xlm","status":"announced",
 "src":{"chain":"ethereum","address":"0x1c7d4b196cb0c7b01d743fbc6116a902379c7238", ...}}
   ^ stored lowercase canonical, even though request used EIP-55 mixed case

$ curl -s 'localhost:3999/api/orders/history?address=0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238'
HTTP 200 -> 1 matching order (valid EIP-55 query)

$ curl -s 'localhost:3999/api/orders/history?address=%20%200x1c7d4b196cb0c7b01d743fbc6116a902379c7238%20'
HTTP 200 -> 1 matching order (whitespace-padded + lowercase query, trimmed + canonicalized)

$ curl -s 'localhost:3999/api/orders/history?address=0x1C7D4B196Cb0C7B01d743Fbc6116a902379C7238'
HTTP 400
{"error":"invalid_address","message":"address has an invalid EIP-55 checksum (expected 0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238)"}

$ curl -s -X POST localhost:3999/api/orders/announce … "srcAddress":"0x1c7D4B196Cb0C7B01d743Fbc6116a902379C723" (39 hex digits)
HTTP 400
{"error":"order_validation_error","message":"srcAddress must be 40 hex digits after 0x (got 39)"}

$ curl -s -X POST localhost:3999/api/orders/announce … "dstAddress":"gdqk…6" (lowercase Stellar)
HTTP 400
{"error":"order_validation_error","message":"dstAddress must be a Stellar account ID (G + 55 base32 characters)"}

$ curl -s 'localhost:3999/api/orders/history?address=GDQK…7AS6'
HTTP 200 -> 1 matching order (Stellar dst side of the same bridge order)
  • Metrics / KPI change → n/a
  • Status table / README change → n/a

Secrets, logging, and PII risk

  • No secrets, private keys, RPC credentials, .env content, wallet mnemonics, or preimages added to the repo
  • No new console.* / logger.* line that prints secrets, preimages, signed payloads, or PII
  • No new Vite/build flag that exposes devtools output in production (the VITE_* and esbuild.drop policy still holds)
  • None of the above — explain why this PR cannot be a secrets / logging risk:

Validation error bodies echo only the public canonical form of the public address the caller itself submitted (e.g. the expected EIP-55 checksum); no preimages, keys, or signed payloads are ever logged or returned.

Public proof links (SCF / investor evidence)

Only required for SCF tranche PRs or investor evidence packs. Otherwise write n/a.

n/a

Breaking change & rollback

  • Breaking change? Yes / No — describe caller impact, data migrations, revert safety:

    Soft break at the API edge. Malformed addresses (mixed-case with a broken EIP-55 checksum, wrong hex length, lowercase Stellar IDs, untrimmable garbage) are now rejected with 400 instead of being stored/queried verbatim. Callers sending all-lowercase or valid EIP-55 addresses are unaffected. No schema change, no migration, no new dependencies. ETH addresses are stored in lowercase canonical form and lookups canonicalize the query, so new and canonical data is always mutually reachable; the coordinator DB is documented as a rebuildable cache of on-chain state, so any pre-existing mixed-case rows (none in this pre-launch testnet deployment) can be lowercased with a one-liner or rebuilt from chain events.

  • Migration or feature flag required? Yes / No — describe the path:

    No. Rollback is a single git revert of the branch: the coordinator returns to verbatim storage/lookup (pre-PR behavior) with no data migration in either direction.

Reviewer checklist (for the PR author to self-verify)

  • PR description and code comments are in English
  • Linked issue or milestone
  • No unrelated drive-by changes (reformatting, dep bumps, etc.)
  • Tests touch the same files as the source change
  • PR is reversible: a single git revert restores prior state

Files in this branch (30: 3 created / 27 modified)

created
  coordinator/test/support/node-sqlite.ts      hasNativeSqlite() probe for runIf skips
  packages/sdk/src/addresses/index.ts          new canonicalization module
  packages/sdk/test/addresses.test.ts          36 tests
modified
  coordinator/src/persistence/db.ts            lazy node:sqlite load + clear Node<22.5 error
  coordinator/src/server/routes/orders.ts      announce/history validation + 400 mapping
  coordinator/src/services/order-service.ts    canonicalize on write/lookup, idempotent secret reveal
  coordinator/test/http-routes.test.ts
  coordinator/test/order-fixtures.test.ts
  coordinator/test/order-metrics.test.ts
  coordinator/test/order-service.test.ts
  coordinator/test/order-transitions.test.ts
  coordinator/test/quote-expiry.test.ts
  coordinator/test/secret-validation.test.ts
  coordinator/test/snapshot.test.ts
  e2e/evm-fixture.ts                           kill hardhat process tree (no leaked nodes)
  frontend/package.json                        pretest builds SDK dist
  frontend/src/App.test.tsx                    networks mock
  frontend/src/components/TokenSelector.tsx    canonical display
  frontend/src/config/networks.ts              Sepolia USDC = 0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238
  packages/sdk/src/assets/capabilities.ts
  packages/sdk/src/assets/index.ts             strict canonical asset matching
  packages/sdk/src/deployment-evidence/index.ts  JSON import attribute for ESM
  packages/sdk/src/index.ts                    export addresses module
  packages/sdk/test/assets.test.ts
  relayer/src/index.ts
  relayer/src/utils.ts                         canonicalization + trim-tolerant validators
  relayer/test/address-utils.test.ts
  scripts/lib/testnet-asset-consistency.mjs    USDC constant
  docs/REVIEW_RESPONSE.md                      section 10

Notes for reviewers

  1. In a fresh clone run pnpm build before pnpm test: the coordinator tests import the SDK's built dist/, and the repo's test scripts don't build it first (pre-existing quirk; CI builds before testing and is unaffected).
  2. Pre-existing deploy gap (disclosed, not fixed here): tsc does not copy coordinator/src/persistence/schema.sql into dist/, so node dist/index.js with SQLite needs a copy step added to the build.

CLOSE #845

@drips-wave

drips-wave Bot commented Sep 3, 2026

Copy link
Copy Markdown

@veloura-dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@anumukul
anumukul merged commit 941a5ef into anumukul:main Sep 4, 2026
3 of 14 checks passed
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.

[DOC-44] Add smart contract gas optimization guide

2 participants