Skip to content

fix(sdk-coin-eth): complete sepeth registration and fix testnet coin resolution - #9433

Merged
rithwik-bitgo merged 1 commit into
masterfrom
CECHO-1721-sepeth-onboarding-fixes
Aug 6, 2026
Merged

fix(sdk-coin-eth): complete sepeth registration and fix testnet coin resolution#9433
rithwik-bitgo merged 1 commit into
masterfrom
CECHO-1721-sepeth-onboarding-fixes

Conversation

@rithwik-bitgo

@rithwik-bitgo rithwik-bitgo commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Register sepeth (Sepolia Ethereum) as an SDK coin — was missing, blocking Sepolia wallet creation (follow-up to feat(statics): add sepolia testnet for ETH #9377, feat(statics): give Sepolia testnet its own underlying asset (sepETH) #9389).
  • Add Networks.test.sepoliasepeth mapping in getErc20TokenConfig() and getErc7984TokenConfig() in statics.
  • Fix base coin resolution for Eth-family testnet tokens in erc20Token.ts/erc7984Token.ts:
    • Erc20Token/Erc7984Token picked their base staticsCoin via coinNames[tokenConfig.network], where coinNames = { Mainnet: 'eth', Testnet: 'hteth' }. Since tokenConfig.network is only ever the generic bucket 'Mainnet' or 'Testnet' (never the specific chain), every testnet token — Holesky, Hoodi, or Sepolia — resolved to the same fixed 'hteth'.
    • This worked by coincidence while hteth was the only Eth testnet with live tokens. With sepeth now active, a Sepolia token would have silently built/signed transactions using hteth's chain data (chainId 17000 instead of 11155111, wrong wallet/forwarder contract addresses).
    • Fix: resolve staticsCoin from tokenConfig.coin instead, which is already set per-network by getErc20TokenConfig()/getErc7984TokenConfig() ('hteth', 'sepeth', etc.).
    • Mainnet is unaffected — tokenConfig.coin is 'eth' for mainnet tokens, same value the old coinNames.Mainnet lookup produced.

Test plan

  • yarn tsc --noEmit clean for sdk-coin-eth and statics
  • sdk-coin-eth full unit suite passing (382 tests)
  • statics tokenConfig-related tests passing
  • Updated register.ts unit test to account for new sepeth base coin registration

Ticket: CECHO-1721

…resolution

Sepolia coin creation was failing because sepeth was never registered as
an SDK coin, and Sepolia ERC20/ERC7984 tokens would have resolved to
hteth's chain data (wrong chainId/contract addresses) since the token
classes hardcoded Testnet lookups to hteth.

Ticket: CECHO-1721
@rithwik-bitgo
rithwik-bitgo requested a review from a team as a code owner August 6, 2026 02:21
@linear-code

linear-code Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

CECHO-1721

@manojkumar138

Copy link
Copy Markdown
Contributor

@claude review

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @manojkumar138's task —— View job


Code Review

Todo List:

  • Read repository guidelines and setup
  • Review core logic changes in erc20Token.ts and erc7984Token.ts
  • Examine new sepeth.ts implementation
  • Check registration updates in register.ts
  • Review test changes
  • Analyze tokenConfig.ts modifications
  • Provide comprehensive review feedback

Summary

This PR successfully addresses the missing Sepolia Ethereum (sepeth) registration and fixes a critical bug in testnet token base coin resolution. The changes are well-structured and solve the exact problems described in the PR description.

Key Changes Analysis

Core Logic Fix (erc20Token.ts & erc7984Token.ts)

Issue Fixed: The original code used Erc20Token.coinNames[tokenConfig.network] where tokenConfig.network is always generic ('Mainnet' or 'Testnet'), causing all testnet tokens to incorrectly resolve to 'hteth' regardless of their actual network.

Solution: Changed to coins.get(tokenConfig.coin) where tokenConfig.coin is already set per-network by getErc20TokenConfig()/getErc7984TokenConfig() ('hteth', 'sepeth', etc.).

This is a critical fix - without it, Sepolia tokens would have used Holesky chain data (chainId 17000 instead of 11155111, wrong contract addresses).

Sepeth Implementation (sepeth.ts)

The new sepeth.ts follows the established pattern:

  • Extends Eth base class ✓
  • Implements createInstance factory method ✓
  • Clean, minimal implementation matching other testnet coins ✓

Registration Updates (register.ts)

  • Properly imports Sepeth class
  • Registers sepeth in both register() and registerWithCoinMap() functions
  • Maintains alphabetical ordering of registrations

Configuration Updates (tokenConfig.ts)

  • Adds Networks.test.sepolia.name'sepeth' mapping in both getErc20TokenConfig() and getErc7984TokenConfig()
  • Maintains consistency with existing patterns
  • Provides proper network-to-coin resolution

Test Updates

  • Updates unit test expectations to account for new sepeth registration
  • Correctly increments expected call count from 4 to 5 base coins
  • Verifies sepeth is properly registered in both test scenarios

Security & Architecture Considerations

  1. No Security Issues: The changes don't introduce any security vulnerabilities
  2. Backwards Compatible: Mainnet functionality unchanged, only fixes testnet behavior
  3. Type Safety: All changes maintain proper TypeScript typing
  4. Pattern Consistency: New code follows established architectural patterns

Minor Observations

  1. Static coinNames Property: The coinNames property in both token classes is now unused after the fix. Consider removing it in a future cleanup PR.

  2. Documentation: Consider updating any documentation that might reference the old behavior, though none appear to be affected in this changeset.

Test Coverage

The PR author mentions running:

  • TypeScript compilation checks ✓
  • Full sdk-coin-eth unit suite (382 tests) ✓
  • statics tokenConfig tests ✓
  • Updated register.ts unit test ✓

Recommendation: APPROVE

This PR:

  • ✅ Fixes a critical bug that would cause incorrect chain data usage for Sepolia tokens
  • ✅ Properly implements missing sepeth registration
  • ✅ Follows established code patterns and conventions
  • ✅ Includes appropriate test updates
  • ✅ Has no breaking changes or security issues
  • ✅ Solves the exact problems described in the Linear ticket

The implementation is clean, well-tested, and addresses a blocking issue for Sepolia wallet creation while preventing silent failures that could lead to transaction errors.


Job run: https://github.com/BitGo/BitGoJS/actions/runs

@rithwik-bitgo
rithwik-bitgo merged commit 581c437 into master Aug 6, 2026
25 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.

2 participants