Skip to content

Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet and Sepolia#908

Open
jose-compu wants to merge 27 commits intothreshold-network:mainfrom
jose-compu:ntt-deploy-fixes
Open

Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet and Sepolia#908
jose-compu wants to merge 27 commits intothreshold-network:mainfrom
jose-compu:ntt-deploy-fixes

Conversation

@jose-compu
Copy link
Copy Markdown
Contributor

@jose-compu jose-compu commented Oct 13, 2025 •
edited
Loading

Ethereum:

  • Proxy: 0xf79b82b345573F7087375ed758eDAa33acCDeCED
  • **Implementation: ** 0x2353A3BCE0703c73F8B1F9e036034AC2BA7feAF6

Sepolia:

  • Proxy: 0x1C805997c6D8Ed2aC38323fd810130c601435f83 (proxy)
  • **Implementation: ** 0x156C78f7c3754D79FA873c942116a0b84846c85d

Key Changes:

  • Contract size optimization - Reduced from 21.6 KiB to 18.9 KiB (fits 24 KiB limit)
    • Removed getUserNonceSequence(address user) getter function to save bytecode
    • Nonce tracking remains fully functional via internal userNonceCounter mapping
  • Gas limit fixes - Added 8M gas limit to handle complex deployment
  • Etherscan V2 API - Updated verification config with chainid=1 parameter
  • RPC provider - Switched to ethereum.publicnode.com for reliable deployment
  • Checksummed addresses - Fixed all contract addresses to proper EIP-55 format
  • Sei deployment infrastructure - Updated cross-chain deployment scripts with working config

Contract Features:

  • Transparent upgradeable proxy deployment
  • NTT Manager integration for cross-chain tBTC transfers
  • Executor service support for automatic destination execution
  • Parameter expiration and workflow management

Nonce System:

  • Internal tracking - Each user has a private nonce counter (userNonceCounter) that increments with each new parameter set
  • Unique nonces - Generated via keccak256(user_address, sequence) to support parallel workflows across multiple users
  • No external getter - The getUserNonceSequence() function was removed during size optimization. Nonce information remains accessible through getUserWorkflowInfo() and getUserWorkflowStatus() for active workflows
  • Workflow management - Users can refresh existing parameters or wait for expiration (default 1 hour) before starting new workflows

Ready for production use on Ethereum Mainnet.

@jose-compu jose-compu changed the title Ntt deploy fixes NTT deploy improvement Oct 13, 2025
@jose-compu jose-compu self-assigned this Oct 15, 2025
@jose-compu jose-compu changed the title NTT deploy improvement chore: Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet Oct 15, 2025
@jose-compu jose-compu added the :shipit: deployment Deployments and CI workflows label Oct 15, 2025
@jose-compu jose-compu changed the title chore: Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet Oct 15, 2025
@jose-compu jose-compu marked this pull request as ready for review October 20, 2025 17:54
piotr-roslaniec
piotr-roslaniec reviewed Oct 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@piotr-roslaniec piotr-roslaniec left a comment

Choose a reason for hiding this comment

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

If you decide to use LLMs in development, please make sure the code and the comments are accurate and that the comments are not redundant.

@jose-compu jose-compu changed the title Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet Deploy and verify L1BTCDepositorNttWithExecutor on Ethereum Mainnet and Sepolia Oct 24, 2025
@jose-compu
feat: Add Sepolia deployment script and update hardhat config
f3434de 
- Add simple-deploy.ts for L1BTCDepositorNttWithExecutor deployment
- Update hardhat.config.ts to use Etherscan API V2 and environment variables
@jose-compu
NTT Paths Configure Script
b6c6024
@jose-compu
format simple-deploy.ts
2b55794
piotr-roslaniec
piotr-roslaniec reviewed Nov 6, 2025
View reviewed changes
piotr-roslaniec
piotr-roslaniec reviewed Nov 6, 2025
View reviewed changes
piotr-roslaniec
piotr-roslaniec reviewed Nov 6, 2025
View reviewed changes
jose-compu and others added 3 commits November 6, 2025 11:44
@jose-compu
Merge branch 'threshold-network:main' into ntt-deploy-fixes
1ee1e7a
@jose-compu
fix(L1BTCDepositorNttWithExecutor): prevent deposit theft via malicio…
8c40f8c 
…us fee parameters (MB-C1)

Add validation in setExecutorParameters() to enforce that feeArgs.payee must match
defaultPlatformFeeRecipient, preventing attackers from redirecting platform fees to
arbitrary addresses. Initialize defaultPlatformFeeRecipient to Threshold Committee
wallet (0x9F6e831c8F8939DC0C830C6e492e7cEf4f9C2F5f) on contract initialization.

Changes:
- Add payee validation in setExecutorParameters() requiring match with defaultPlatformFeeRecipient
- Set defaultPlatformFeeRecipient to 0x9F6e831c8F8939DC0C830C6e492e7cEf4f9C2F5f in initialize()
- Add comprehensive test suite demonstrating vulnerability and fix effectiveness
- Update existing tests to align with new validation requirements

The vulnerability allowed malicious actors to set arbitrary fee recipients when calling
setExecutorParameters(), enabling them to steal platform fees from legitimate deposits.
This fix ensures all fees are directed to the protocol-designated recipient.

Tests: 164 passing in L1BTCDepositorNttWithExecutor test suite
@jose-compu
fix(L1BTCDepositorNttWithExecutor): prevent deposit loss through para…
92b8efa 
…meter validation (MB-C2)

Add comprehensive parameter validation to prevent deposit loss scenarios:
1. Extract and validate embedded expiry timestamp from signed quotes
2. Validate total payment includes both executor cost and NTT delivery price
3. Remove timestamp refresh to prevent artificial quote expiry extension

Changes:
- Add _validateAndExtractQuoteExpiry() to extract and validate quote expiry at byte offset 60
- Enhance _transferTbtcWithExecutor() to validate msg.value >= executorArgs.value + nttDeliveryPrice
- Remove timestamp refresh in setExecutorParameters() to preserve original expiration tracking
- Add comprehensive C2 vulnerability test suite with 5 tests
- Add contract size validation test confirming 19.6 KB (within 24 KB Ethereum limit)

The vulnerability allowed deposits to be finalized with expired quotes or insufficient payment,
resulting in cross-chain transfer failures and loss of deposited funds. This fix ensures deposits
only finalize when they can actually be delivered cross-chain.

Tests: 169 passing (includes C1 + C2 vulnerability tests)
Contract size: 19.6 KB (80% of 24 KB Ethereum mainnet limit)
@jose-compu
feat: MB-H1, MB-H2, MB-H3 - Create AbstractL1BTCDepositorV2 with refu…
3fb047f 
…nd vulnerability fixes

Introduces AbstractL1BTCDepositorV2 to address three refund-related vulnerabilities
in deposit finalization, isolated to NTT contracts only.

MB-H1 (Refund Order): Finalization refund calculated before initialization refund
to prevent gas double-counting attacks via malicious receiver fallbacks.

MB-H2 (Refund Blocking): Added try-catch protection and claimReimbursement() function
to prevent deposit blocking when ReimbursementPool is unavailable.

MB-H3 (Gas Price Arbitrage): GasReimbursement struct stores exact wei amounts locked
at initialization gas price to prevent arbitrage between init and finalize.

L1BTCDepositorNttWithExecutor now inherits from V2 while other depositor implementations
remain on AbstractL1BTCDepositor unchanged.
@jose-compu
fix: MB-M2 - Query underlying NTT manager for delivery prices
b0c4dff 
The deployed Wormhole NttManagerWithExecutor contract doesn't expose
quoteDeliveryPrice(). Fixed by querying the underlying NTT manager
directly in all quote functions.

Changes:
- quoteFinalizeDeposit() now queries INttManager(underlyingNttManager)
- quoteFinalizeDeposit(uint16) now queries INttManager(underlyingNttManager)
- Both calculate total cost as: nttDeliveryPrice + executorArgs.value

Compatible with deployed Wormhole contracts. All 183 tests passing.
@jose-compu
docs: MB-L1 - Fix MAX_BPS documentation, 10000 represents 10% not 100%
dacf177 
NttManagerWithExecutor uses 100000 divisor. Updated all comments and error
messages to correctly state MAX_BPS = 10000 = 10%.
@jose-compu
refactor: MB-L2 - Code quality improvements
e82d98c 
1. Add gas limit validation in setDefaultParameters()
2. Add ExecutorParametersCleared event for monitoring
3. Remove redundant signed quote length checks

Contract size reduced by 0.147 KB through eliminating duplicate validations.
@jose-compu
fix: MB-L3 - Lock parameter expiration at creation time
0258ab1 
Store absolute expiryTimestamp in ExecutorParameterSet to prevent governance
changes from retroactively affecting existing parameters. Adds expiryTimestamp
field set to block.timestamp + parameterExpirationTime at creation.
antomor
antomor reviewed Dec 10, 2025
View reviewed changes
antomor
antomor reviewed Dec 10, 2025
View reviewed changes
antomor
antomor reviewed Dec 10, 2025
View reviewed changes
antomor
antomor reviewed Dec 10, 2025
View reviewed changes
antomor
antomor reviewed Dec 10, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

@jose-blockchain I haven't focussed the review on the sc as I don't have context for the reasons behind those changes, but I've left some comments on the deployment scripts. Apart from them, I'd try to avoid any as much as possible to get the most from ts. Furthermore, although not linked to this PR, I could see from npm i that some packages require security fixes that could be quickly applied with npm audit fix.

jose-compu and others added 13 commits December 24, 2025 14:53
@jose-compu
Copy link
Copy Markdown
Contributor Author

jose-compu commented Jan 5, 2026 •
edited
Loading

@antomor thanks a lot! For merging, it seems I still need code review from leo or piotr still as they are code owners

@jose-compu jose-compu requested a review from antomor January 5, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lrsaturnino lrsaturnino lrsaturnino approved these changes

@pdyraga pdyraga Awaiting requested review from pdyraga pdyraga is a code owner

@lukasz-zimnoch lukasz-zimnoch Awaiting requested review from lukasz-zimnoch lukasz-zimnoch is a code owner

@Shadowfiend Shadowfiend Awaiting requested review from Shadowfiend Shadowfiend is a code owner

@nkuba nkuba Awaiting requested review from nkuba nkuba is a code owner

@evandrosaturnino evandrosaturnino Awaiting requested review from evandrosaturnino

@derekpierre derekpierre Awaiting requested review from derekpierre

@piotr-roslaniec piotr-roslaniec Awaiting requested review from piotr-roslaniec piotr-roslaniec is a code owner

@antomor antomor Awaiting requested review from antomor

Assignees

@jose-compu jose-compu

Labels

:shipit: deployment Deployments and CI workflows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jose-compu @lrsaturnino @antomor @piotr-roslaniec