From af0b3dbf3f926027bc5b94f586d341293495640f Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 14:47:04 +0100 Subject: [PATCH 01/10] Return initial deposit amount from _initializeDeposit function Integrators may be interested in adding more fees to the deposit amount. For this reqson it may be cleanest to calculate im based on the initial deposit amount. By returning the value here they will avoid reading from the storage again. --- .../contracts/integrator/TBTCDepositorProxy.sol | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index ecb75de1b..fc1a8c1c8 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -167,7 +167,10 @@ abstract contract TBTCDepositorProxy { /// for the deposit and calling the `onDepositFinalized` callback /// function. /// @param depositKey Deposit key identifying the deposit. - /// @return tbtcAmount Approximate amount of TBTC minted for the deposit. + /// @return initialDepositAmount Amount of funding transaction deposit. In + /// TBTC token decimals precision. + /// @return tbtcAmount Approximate amount of TBTC minted for the deposit. In + /// TBTC token decimals precision. /// @return extraData 32-byte deposit extra data. /// @dev Requirements: /// - The deposit must be initialized but not finalized @@ -178,9 +181,15 @@ abstract contract TBTCDepositorProxy { /// approximation. See documentation of the `calculateTbtcAmount` /// responsible for calculating this value for more details. // slither-disable-next-line dead-code - function _finalizeDeposit(uint256 depositKey) + function _finalizeDeposit( + uint256 depositKey + ) internal - returns (uint256 tbtcAmount, bytes32 extraData) + returns ( + uint256 initialDepositAmount, + uint256 tbtcAmount, + bytes32 extraData + ) { require(pendingDeposits[depositKey], "Deposit not initialized"); @@ -204,6 +213,8 @@ abstract contract TBTCDepositorProxy { // slither-disable-next-line reentrancy-no-eth delete pendingDeposits[depositKey]; + initialDepositAmount = deposit.amount * SATOSHI_MULTIPLIER; + tbtcAmount = _calculateTbtcAmount(deposit.amount, deposit.treasuryFee); // slither-disable-next-line reentrancy-events From b50b296405bc13d513b9280df321fa39e5a4f43b Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 14:49:32 +0100 Subject: [PATCH 02/10] Make some mocked functions public By changing these functions to public the mock contract can be used by integrators to tweak the contracts to their expectations. --- solidity/contracts/test/TestTBTCDepositorProxy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index dd611503c..5f1fe41d3 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -89,7 +89,7 @@ contract MockBridge is IBridge { emit DepositRevealed(depositKey); } - function sweepDeposit(uint256 depositKey) external { + function sweepDeposit(uint256 depositKey) public { require(_deposits[depositKey].revealedAt != 0, "Deposit not revealed"); require(_deposits[depositKey].sweptAt == 0, "Deposit already swept"); /* solhint-disable-next-line not-rely-on-time */ @@ -152,7 +152,7 @@ contract MockTBTCVault is ITBTCVault { _requests[depositKey].requestedAt = uint64(block.timestamp); } - function finalizeOptimisticMintingRequest(uint256 depositKey) external { + function finalizeOptimisticMintingRequest(uint256 depositKey) public { require( _requests[depositKey].requestedAt != 0, "Request does not exist" From d5b047fbcc787870cfa7932ecc564ff64eb65e7d Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 14:52:19 +0100 Subject: [PATCH 03/10] Make treasury fee configurable in the MockBridge contract The fee being configurable may be useful for integrators using the stub contract in tests. --- solidity/contracts/test/TestTBTCDepositorProxy.sol | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 5f1fe41d3..6fbe3798c 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -44,6 +44,7 @@ contract MockBridge is IBridge { mapping(uint256 => IBridgeTypes.DepositRequest) internal _deposits; + uint64 internal _depositTreasuryFeeDivisor = 100; // 1/100 == 100 bps == 1% == 0.01 uint64 internal _depositTxMaxFee = 1 * 1e7; // 0.1 BTC event DepositRevealed(uint256 depositKey); @@ -80,7 +81,9 @@ contract MockBridge is IBridge { /* solhint-disable-next-line not-rely-on-time */ request.revealedAt = uint32(block.timestamp); request.vault = reveal.vault; - request.treasuryFee = uint64(1 * 1e8); // 1 BTC + request.treasuryFee = _depositTreasuryFeeDivisor > 0 + ? fundingOutputAmount / _depositTreasuryFeeDivisor + : 0; request.sweptAt = 0; request.extraData = extraData; @@ -120,6 +123,10 @@ contract MockBridge is IBridge { depositRevealAheadPeriod = 0; } + function setDepositTreasuryFeeDivisor(uint64 value) external { + _depositTreasuryFeeDivisor = value; + } + function setDepositTxMaxFee(uint64 value) external { _depositTxMaxFee = value; } From b00ee8b6be064ce63850b16ab3a51c1675d20996 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 14:53:52 +0100 Subject: [PATCH 04/10] Extract funding output amount from transaction in MockBridge contract Using the value set in the transaction gives integrator a flexibility to test with different deposit values than 10 BTC. --- solidity/contracts/test/TestTBTCDepositorProxy.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 6fbe3798c..8f0566844 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -74,10 +74,16 @@ contract MockBridge is IBridge { "Deposit already revealed" ); + bytes memory fundingOutput = fundingTx + .outputVector + .extractOutputAtIndex(reveal.fundingOutputIndex); + + uint64 fundingOutputAmount = fundingOutput.extractValue(); + IBridgeTypes.DepositRequest memory request; request.depositor = msg.sender; - request.amount = uint64(10 * 1e8); // 10 BTC + request.amount = fundingOutputAmount; /* solhint-disable-next-line not-rely-on-time */ request.revealedAt = uint32(block.timestamp); request.vault = reveal.vault; From dad0e7b53d7699d8cf782e667cdfc2934be06d14 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 15:41:48 +0100 Subject: [PATCH 05/10] Update test mock for finalizeDeposit function New return parameter was covered in the mock to test the value. --- .../contracts/test/TestTBTCDepositorProxy.sol | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 8f0566844..9909aeeff 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -11,7 +11,11 @@ import "../integrator/ITBTCVault.sol"; contract TestTBTCDepositorProxy is TBTCDepositorProxy { event InitializeDepositReturned(uint256 depositKey); - event FinalizeDepositReturned(uint256 tbtcAmount, bytes32 extraData); + event FinalizeDepositReturned( + uint256 initialDepositAmount, + uint256 tbtcAmount, + bytes32 extraData + ); function initialize(address _bridge, address _tbtcVault) external { __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); @@ -27,8 +31,16 @@ contract TestTBTCDepositorProxy is TBTCDepositorProxy { } function finalizeDepositPublic(uint256 depositKey) external { - (uint256 tbtcAmount, bytes32 extraData) = _finalizeDeposit(depositKey); - emit FinalizeDepositReturned(tbtcAmount, extraData); + ( + uint256 initialDepositAmount, + uint256 tbtcAmount, + bytes32 extraData + ) = _finalizeDeposit(depositKey); + emit FinalizeDepositReturned( + initialDepositAmount, + tbtcAmount, + extraData + ); } function calculateTbtcAmountPublic( From 64528410730b879e35f0f41c6e1e213d635786bf Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 15:42:44 +0100 Subject: [PATCH 06/10] Update default values for fees calculation in MockBridge contract --- solidity/contracts/test/TestTBTCDepositorProxy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 9909aeeff..2fe6d452e 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -56,8 +56,8 @@ contract MockBridge is IBridge { mapping(uint256 => IBridgeTypes.DepositRequest) internal _deposits; - uint64 internal _depositTreasuryFeeDivisor = 100; // 1/100 == 100 bps == 1% == 0.01 - uint64 internal _depositTxMaxFee = 1 * 1e7; // 0.1 BTC + uint64 internal _depositTreasuryFeeDivisor = 50; // 1/50 == 100 bps == 2% == 0.02 + uint64 internal _depositTxMaxFee = 1000; // 1000 satoshi = 0.00001 BTC event DepositRevealed(uint256 depositKey); From c28dc2cffb50a59dc5f55ea7634b6b8211e9608b Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 15:40:58 +0100 Subject: [PATCH 07/10] Update unit tests for TBTCDepositorProxy contract Update calculations of amounts to match the changes made in the mock contracts. The biggest difference is that the funding transaction is no longer hardcoded in the MockBridge contract, but the value in the fixture transaction is used. --- .../integrator/TBTCDepositorProxy.test.ts | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/TBTCDepositorProxy.test.ts index f2c39d457..8ed86e6a7 100644 --- a/solidity/test/integrator/TBTCDepositorProxy.test.ts +++ b/solidity/test/integrator/TBTCDepositorProxy.test.ts @@ -214,13 +214,14 @@ describe("TBTCDepositorProxy", () => { context("when deposit is finalized by the Bridge", () => { // The expected tbtcAmount is calculated as follows: // - // - Deposit amount = 10 BTC (hardcoded in MockBridge) - // - Treasury fee = 1 BTC (hardcoded in MockBridge) - // - Optimistic minting fee = 1% (default value used in MockTBTCVault) - // - Transaction max fee = 0.1 BTC (default value used in MockBridge) + // - Deposit amount = 10000 satoshi (hardcoded in funding transaction fixture) + // - Treasury fee = 2% (default value used in MockBridge) + // - Optimistic minting fee = 1% (default value used in MockTBTCVault) + // - Transaction max fee = 1000 satoshi (default value used in MockBridge) // - // ((10 BTC - 1 BTC) * 0.99) - 0.1 BTC = 8.81 BTC = 8.81 * 1e8 sat = 8.81 * 1e18 TBTC - const expectedTbtcAmount = to1ePrecision(881, 16).toString() + // ((10000 sat - 200 sat) * 0.99) - 2000 sat = 8702 sat = 8702 * 1e10 TBTC + const expectedInitialDepositAmount = to1ePrecision(10000, 10) + const expectedTbtcAmount = to1ePrecision(8702, 10).toString() context("when the deposit is swept", () => { let tx: ContractTransaction @@ -259,7 +260,11 @@ describe("TBTCDepositorProxy", () => { it("should return proper values", async () => { await expect(tx) .to.emit(depositorProxy, "FinalizeDepositReturned") - .withArgs(expectedTbtcAmount, fixture.extraData) + .withArgs( + expectedInitialDepositAmount, + expectedTbtcAmount, + fixture.extraData + ) }) }) @@ -306,7 +311,11 @@ describe("TBTCDepositorProxy", () => { it("should return proper values", async () => { await expect(tx) .to.emit(depositorProxy, "FinalizeDepositReturned") - .withArgs(expectedTbtcAmount, fixture.extraData) + .withArgs( + expectedInitialDepositAmount, + expectedTbtcAmount, + fixture.extraData + ) }) }) }) @@ -314,6 +323,17 @@ describe("TBTCDepositorProxy", () => { }) describe("_calculateTbtcAmount", () => { + before(async () => { + await createSnapshot() + + // Set the transaction max fee to 0.1 BTC. + await bridge.setDepositTxMaxFee(10000000) + }) + + after(async () => { + await restoreSnapshot() + }) + context("when all fees are non-zero", () => { it("should return the correct amount", async () => { const depositAmount = to1ePrecision(10, 8) // 10 BTC @@ -324,7 +344,7 @@ describe("TBTCDepositorProxy", () => { // - Deposit amount = 10 BTC // - Treasury fee = 1 BTC // - Optimistic minting fee = 1% (default value used in MockTBTCVault) - // - Transaction max fee = 0.1 BTC (default value used in MockBridge) + // - Transaction max fee = 0.1 BTC (set in MockBridge) // // ((10 BTC - 1 BTC) * 0.99) - 0.1 BTC = 8.81 BTC = 8.81 * 1e8 sat = 8.81 * 1e18 TBTC const expectedTbtcAmount = to1ePrecision(881, 16) @@ -386,7 +406,7 @@ describe("TBTCDepositorProxy", () => { // - Deposit amount = 10 BTC // - Treasury fee = 0 BTC // - Optimistic minting fee = 1% (default value used in MockTBTCVault) - // - Transaction max fee = 0.1 BTC (default value used in MockBridge) + // - Transaction max fee = 0.1 BTC (set in MockBridge) // // ((10 BTC - 0 BTC) * 0.99) - 0.1 BTC = 9.8 BTC = 9.8 * 1e8 sat = 9.8 * 1e18 TBTC const expectedTbtcAmount = to1ePrecision(98, 17) @@ -421,7 +441,7 @@ describe("TBTCDepositorProxy", () => { // - Deposit amount = 10 BTC // - Treasury fee = 1 BTC // - Optimistic minting fee = 0% (set in MockTBTCVault) - // - Transaction max fee = 0.1 BTC (default value used in MockBridge) + // - Transaction max fee = 0.1 BTC (set in MockBridge) // // ((10 BTC - 1 BTC) * 1) - 0.1 BTC = 8.9 BTC = 8.9 * 1e8 sat = 8.9 * 1e18 TBTC const expectedTbtcAmount = to1ePrecision(89, 17) From 6fc1537aa5940146e21f73f1c17c8b56be8238d5 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 15:44:02 +0100 Subject: [PATCH 08/10] Define .nvmrc file with node version The file helps with switching expected node version between the workspaces. lts/hydrogen is node v18, which is the highest node version suported by this module. --- solidity/.nvmrc | 1 + 1 file changed, 1 insertion(+) create mode 100644 solidity/.nvmrc diff --git a/solidity/.nvmrc b/solidity/.nvmrc new file mode 100644 index 000000000..a77793ecc --- /dev/null +++ b/solidity/.nvmrc @@ -0,0 +1 @@ +lts/hydrogen From c0c9e07970c016dfd309c8c4fb416712a29da223 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 15:55:50 +0100 Subject: [PATCH 09/10] Fix code formatting Result of running `yarn format:fix` command. --- .../integrator/TBTCDepositorProxy.sol | 4 +--- .../integrator/TBTCDepositorProxy.test.ts | 20 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index fc1a8c1c8..5a168d19e 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -181,9 +181,7 @@ abstract contract TBTCDepositorProxy { /// approximation. See documentation of the `calculateTbtcAmount` /// responsible for calculating this value for more details. // slither-disable-next-line dead-code - function _finalizeDeposit( - uint256 depositKey - ) + function _finalizeDeposit(uint256 depositKey) internal returns ( uint256 initialDepositAmount, diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/TBTCDepositorProxy.test.ts index 8ed86e6a7..e40cde5d3 100644 --- a/solidity/test/integrator/TBTCDepositorProxy.test.ts +++ b/solidity/test/integrator/TBTCDepositorProxy.test.ts @@ -323,17 +323,17 @@ describe("TBTCDepositorProxy", () => { }) describe("_calculateTbtcAmount", () => { - before(async () => { - await createSnapshot() - - // Set the transaction max fee to 0.1 BTC. - await bridge.setDepositTxMaxFee(10000000) - }) + before(async () => { + await createSnapshot() + + // Set the transaction max fee to 0.1 BTC. + await bridge.setDepositTxMaxFee(10000000) + }) + + after(async () => { + await restoreSnapshot() + }) - after(async () => { - await restoreSnapshot() - }) - context("when all fees are non-zero", () => { it("should return the correct amount", async () => { const depositAmount = to1ePrecision(10, 8) // 10 BTC From 4d74ad9fa082a4e2cf0a9a91d45d5b6b03a0e556 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Thu, 1 Feb 2024 18:36:47 +0100 Subject: [PATCH 10/10] Update example of _finalizeDeposit usage --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 5a168d19e..f988d1208 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -64,7 +64,11 @@ import "./ITBTCVault.sol"; /// } /// /// function finalizeProcess(uint256 depositKey) external { -/// (uint256 tbtcAmount, bytes32 extraData) = _finalizeDeposit(depositKey); +/// ( +/// uint256 initialDepositAmount, +/// uint256 tbtcAmount, +/// bytes32 extraData +/// ) = _finalizeDeposit(depositKey); /// /// // Do something with the minted TBTC using context /// // embedded in the extraData.