From 7e347dced1c7205c6f66b40e46f9b9a166602971 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Fri, 26 Jan 2024 17:11:51 +0100 Subject: [PATCH 01/22] Rough draft of the abstract `DepositorProxy` contract --- solidity/contracts/bridge/DepositorProxy.sol | 151 +++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 solidity/contracts/bridge/DepositorProxy.sol diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/bridge/DepositorProxy.sol new file mode 100644 index 000000000..a2de62165 --- /dev/null +++ b/solidity/contracts/bridge/DepositorProxy.sol @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-3.0-only + +// ██████████████ ▐████▌ ██████████████ +// ██████████████ ▐████▌ ██████████████ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ██████████████ ▐████▌ ██████████████ +// ██████████████ ▐████▌ ██████████████ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ + +pragma solidity 0.8.17; + +import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; + +import "./BitcoinTx.sol"; +import "./Bridge.sol"; +import "./Deposit.sol"; +import "./../vault/TBTCVault.sol"; +import "./../vault/TBTCOptimisticMinting.sol"; +import "./../token/TBTC.sol"; + +// TODO: Make it safe for upgradeable contracts. +// TODO: Document the contract. +// TODO: Move to another directory? +abstract contract DepositorProxy { + using BTCUtils for bytes; + + /// @notice Multiplier to convert satoshi to TBTC token units. + uint256 public constant SATOSHI_MULTIPLIER = 10**10; + + Bridge public bridge; + TBTCVault public tbtcVault; + TBTC public tbtcToken; + + mapping(uint256 => bool) public pendingDeposits; + + event DepositInitialized( + uint256 indexed depositKey, + uint32 initiatedAt + ); + + event DepositFinalized( + uint256 indexed depositKey, + uint256 tbtcAmount, + uint32 finalizedAt + ); + + constructor(address _bridge, address _tbtcVault, address _tbtcToken) { + bridge = Bridge(_bridge); + tbtcVault = TBTCVault(_tbtcVault); + tbtcToken = TBTC(_tbtcToken); + } + + function initializeDeposit( + BitcoinTx.Info calldata fundingTx, + Deposit.DepositRevealInfo calldata reveal, + bytes32 extraData + ) internal { + require(reveal.vault == address(tbtcVault), "Vault address mismatch"); + + uint256 depositKey = calculateDepositKey( + calculateBitcoinTxHash(fundingTx), + reveal.fundingOutputIndex + ); + + pendingDeposits[depositKey] = true; + + emit DepositInitialized( + depositKey, + /* solhint-disable-next-line not-rely-on-time */ + uint32(block.timestamp) + ); + + // The Bridge does not allow to reveal the same deposit twice and + // revealed deposits stay there forever. The transaction will revert + // if the deposit has already been revealed so, there is no need to do + // an explicit check here. + bridge.revealDepositWithExtraData(fundingTx, reveal, extraData); + } + + function finalizeDeposit(uint256 depositKey) internal { + require(pendingDeposits[depositKey], "Deposit not initialized"); + + Deposit.DepositRequest memory deposit = bridge.deposits(depositKey); + + bool minted = deposit.sweptAt != 0 || + tbtcVault.optimisticMintingRequests(depositKey).finalizedAt != 0; + + require(minted, "Deposit not finalized by the bridge"); + + // We can safely delete the deposit from the pending deposits mapping. + // This deposit cannot be initialized again because the bridge does not + // allow to reveal the same deposit twice. + delete pendingDeposits[depositKey]; + + // Both deposit amount and treasury fee are in the 1e8 satoshi precision. + // We need to convert them to the 1e18 TBTC precision. + uint256 amountSubTreasury = (deposit.amount - deposit.treasuryFee) * + SATOSHI_MULTIPLIER; + + uint256 omFeeDivisor = tbtcVault.optimisticMintingFeeDivisor(); + uint256 omFee = omFeeDivisor > 0 ? (amountSubTreasury / omFeeDivisor) : 0; + + // The deposit transaction max fee is in the 1e8 satoshi precision. + // We need to convert them to the 1e18 TBTC precision. + (,,uint64 depositTxMaxFee,) = bridge.depositParameters(); + uint256 txMaxFee = depositTxMaxFee * SATOSHI_MULTIPLIER; + + uint256 tbtcAmount = amountSubTreasury - omFee - txMaxFee; + + emit DepositFinalized( + depositKey, + tbtcAmount, + /* solhint-disable-next-line not-rely-on-time */ + uint32(block.timestamp) + ); + + onDepositFinalized(depositKey, tbtcAmount, deposit.extraData); + } + + function onDepositFinalized( + uint256 depositKey, + uint256 tbtcAmount, + bytes32 extraData + ) internal virtual; + + function calculateDepositKey( + bytes32 fundingTxHash, + uint32 fundingOutputIndex + ) internal pure returns (uint256) { + return uint256( + keccak256(abi.encodePacked(fundingTxHash, fundingOutputIndex)) + ); + } + + function calculateBitcoinTxHash( + BitcoinTx.Info calldata txInfo + ) internal pure returns (bytes32) { + return abi.encodePacked( + txInfo.version, + txInfo.inputVector, + txInfo.outputVector, + txInfo.locktime + ).hash256View(); + } +} \ No newline at end of file From 088b78f71cc2fb35ecae757b5ce5df0df8c94afb Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 13:25:07 +0100 Subject: [PATCH 02/22] s/initiatedAt/initializedAt within `DepositInitialized` evebt --- solidity/contracts/bridge/DepositorProxy.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/bridge/DepositorProxy.sol index a2de62165..c28ef5445 100644 --- a/solidity/contracts/bridge/DepositorProxy.sol +++ b/solidity/contracts/bridge/DepositorProxy.sol @@ -41,7 +41,7 @@ abstract contract DepositorProxy { event DepositInitialized( uint256 indexed depositKey, - uint32 initiatedAt + uint32 initializedAt ); event DepositFinalized( From 47dd043b755ccb7a23dca0e5760bc15aade95b24 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 13:46:12 +0100 Subject: [PATCH 03/22] Explain the reason of `delete pendingDeposits[depositKey]` in a better way --- solidity/contracts/bridge/DepositorProxy.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/bridge/DepositorProxy.sol index c28ef5445..1527e3750 100644 --- a/solidity/contracts/bridge/DepositorProxy.sol +++ b/solidity/contracts/bridge/DepositorProxy.sol @@ -95,7 +95,9 @@ abstract contract DepositorProxy { // We can safely delete the deposit from the pending deposits mapping. // This deposit cannot be initialized again because the bridge does not - // allow to reveal the same deposit twice. + // allow to reveal the same deposit twice. Deleting the deposit from + // the mapping will also prevent the finalizeDeposit function from + // being called again for the same deposit. delete pendingDeposits[depositKey]; // Both deposit amount and treasury fee are in the 1e8 satoshi precision. From 0bb665fc85e0ee8448b1e2a4bb87f88e7e9b409b Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 14:06:13 +0100 Subject: [PATCH 04/22] Extract `calculateTbtcAmount` function Here we extract the tbtc amount calculation logic to a separate `calculateTbtcAmount` function. We are making it `virtual` to allow child contracts override it if they need to do so. --- solidity/contracts/bridge/DepositorProxy.sol | 32 +++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/bridge/DepositorProxy.sol index 1527e3750..5a85f94ad 100644 --- a/solidity/contracts/bridge/DepositorProxy.sol +++ b/solidity/contracts/bridge/DepositorProxy.sol @@ -100,9 +100,28 @@ abstract contract DepositorProxy { // being called again for the same deposit. delete pendingDeposits[depositKey]; + uint256 tbtcAmount = calculateTbtcAmount( + deposit.amount, + deposit.treasuryFee + ); + + emit DepositFinalized( + depositKey, + tbtcAmount, + /* solhint-disable-next-line not-rely-on-time */ + uint32(block.timestamp) + ); + + onDepositFinalized(depositKey, tbtcAmount, deposit.extraData); + } + + function calculateTbtcAmount( + uint64 depositAmountSat, + uint64 depositTreasuryFeeSat + ) internal virtual view returns (uint256) { // Both deposit amount and treasury fee are in the 1e8 satoshi precision. // We need to convert them to the 1e18 TBTC precision. - uint256 amountSubTreasury = (deposit.amount - deposit.treasuryFee) * + uint256 amountSubTreasury = (depositAmountSat - depositTreasuryFeeSat) * SATOSHI_MULTIPLIER; uint256 omFeeDivisor = tbtcVault.optimisticMintingFeeDivisor(); @@ -113,16 +132,7 @@ abstract contract DepositorProxy { (,,uint64 depositTxMaxFee,) = bridge.depositParameters(); uint256 txMaxFee = depositTxMaxFee * SATOSHI_MULTIPLIER; - uint256 tbtcAmount = amountSubTreasury - omFee - txMaxFee; - - emit DepositFinalized( - depositKey, - tbtcAmount, - /* solhint-disable-next-line not-rely-on-time */ - uint32(block.timestamp) - ); - - onDepositFinalized(depositKey, tbtcAmount, deposit.extraData); + return amountSubTreasury - omFee - txMaxFee; } function onDepositFinalized( From 8c068d4f1cda351ce76eb84beda3416f25cdece5 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 14:07:01 +0100 Subject: [PATCH 05/22] Remove reference to the TBTC token contract This reference is not needed at this level as it will be used directly by child contracts. --- solidity/contracts/bridge/DepositorProxy.sol | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/bridge/DepositorProxy.sol index 5a85f94ad..f175fc460 100644 --- a/solidity/contracts/bridge/DepositorProxy.sol +++ b/solidity/contracts/bridge/DepositorProxy.sol @@ -22,7 +22,6 @@ import "./Bridge.sol"; import "./Deposit.sol"; import "./../vault/TBTCVault.sol"; import "./../vault/TBTCOptimisticMinting.sol"; -import "./../token/TBTC.sol"; // TODO: Make it safe for upgradeable contracts. // TODO: Document the contract. @@ -35,7 +34,6 @@ abstract contract DepositorProxy { Bridge public bridge; TBTCVault public tbtcVault; - TBTC public tbtcToken; mapping(uint256 => bool) public pendingDeposits; @@ -50,10 +48,9 @@ abstract contract DepositorProxy { uint32 finalizedAt ); - constructor(address _bridge, address _tbtcVault, address _tbtcToken) { + constructor(address _bridge, address _tbtcVault) { bridge = Bridge(_bridge); tbtcVault = TBTCVault(_tbtcVault); - tbtcToken = TBTC(_tbtcToken); } function initializeDeposit( From fb2acde8a25b90a70398ac6311ccf523faa786de Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 14:07:55 +0100 Subject: [PATCH 06/22] Return deposit key from `initializeDeposit` function Returning deposit key can be useful for child contracts using this function. --- solidity/contracts/bridge/DepositorProxy.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/bridge/DepositorProxy.sol index f175fc460..578f7721e 100644 --- a/solidity/contracts/bridge/DepositorProxy.sol +++ b/solidity/contracts/bridge/DepositorProxy.sol @@ -57,7 +57,7 @@ abstract contract DepositorProxy { BitcoinTx.Info calldata fundingTx, Deposit.DepositRevealInfo calldata reveal, bytes32 extraData - ) internal { + ) internal returns (uint256) { require(reveal.vault == address(tbtcVault), "Vault address mismatch"); uint256 depositKey = calculateDepositKey( @@ -78,6 +78,8 @@ abstract contract DepositorProxy { // if the deposit has already been revealed so, there is no need to do // an explicit check here. bridge.revealDepositWithExtraData(fundingTx, reveal, extraData); + + return depositKey; } function finalizeDeposit(uint256 depositKey) internal { From 8d183bff828610761d23d386406bed4d6d648488 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 14:10:32 +0100 Subject: [PATCH 07/22] Move and rename `DepositorProxy` contract --- .../TBTCDepositorProxy.sol} | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) rename solidity/contracts/{bridge/DepositorProxy.sol => integrator/TBTCDepositorProxy.sol} (97%) diff --git a/solidity/contracts/bridge/DepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol similarity index 97% rename from solidity/contracts/bridge/DepositorProxy.sol rename to solidity/contracts/integrator/TBTCDepositorProxy.sol index 578f7721e..0bc4fed2f 100644 --- a/solidity/contracts/bridge/DepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -17,16 +17,15 @@ pragma solidity 0.8.17; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; -import "./BitcoinTx.sol"; -import "./Bridge.sol"; -import "./Deposit.sol"; +import "./../bridge/BitcoinTx.sol"; +import "./../bridge/Bridge.sol"; +import "./../bridge/Deposit.sol"; import "./../vault/TBTCVault.sol"; import "./../vault/TBTCOptimisticMinting.sol"; // TODO: Make it safe for upgradeable contracts. // TODO: Document the contract. -// TODO: Move to another directory? -abstract contract DepositorProxy { +abstract contract TBTCDepositorProxy { using BTCUtils for bytes; /// @notice Multiplier to convert satoshi to TBTC token units. From 7dc4048a0b91a9523690d23574a6fa3838d5f540 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 15:38:51 +0100 Subject: [PATCH 08/22] Make `TBTCDepositorProxy` safe for upgradable child contracts Here we make the abstract contract upgrade-friendly by replacing the constructor with an initializer and adding a storage gap. --- .../integrator/TBTCDepositorProxy.sol | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 0bc4fed2f..53fe00a14 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -16,6 +16,7 @@ pragma solidity 0.8.17; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "./../bridge/BitcoinTx.sol"; import "./../bridge/Bridge.sol"; @@ -23,9 +24,8 @@ import "./../bridge/Deposit.sol"; import "./../vault/TBTCVault.sol"; import "./../vault/TBTCOptimisticMinting.sol"; -// TODO: Make it safe for upgradeable contracts. // TODO: Document the contract. -abstract contract TBTCDepositorProxy { +abstract contract TBTCDepositorProxy is Initializable { using BTCUtils for bytes; /// @notice Multiplier to convert satoshi to TBTC token units. @@ -36,6 +36,14 @@ abstract contract TBTCDepositorProxy { mapping(uint256 => bool) public pendingDeposits; + // Reserved storage space that allows adding more variables without affecting + // the storage layout of the child contracts. The convention from OpenZeppelin + // suggests the storage space should add up to 50 slots. If more variables are + // added in the upcoming versions one need to reduce the array size accordingly. + // See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps + // slither-disable-next-line unused-state + uint256[47] private __gap; + event DepositInitialized( uint256 indexed depositKey, uint32 initializedAt @@ -47,7 +55,13 @@ abstract contract TBTCDepositorProxy { uint32 finalizedAt ); - constructor(address _bridge, address _tbtcVault) { + function __TBTCDepositorProxy_initialize( + address _bridge, + address _tbtcVault + ) internal onlyInitializing { + require(_bridge != address(0), "Bridge address cannot be zero"); + require(_tbtcVault != address(0), "TBTCVault address cannot be zero"); + bridge = Bridge(_bridge); tbtcVault = TBTCVault(_tbtcVault); } From 535c0618fbfe253d7c9cc637ed8a816d0a819690 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 18:07:05 +0100 Subject: [PATCH 09/22] Documentation for the `TBTCDepositorProxy` contract --- .../integrator/TBTCDepositorProxy.sol | 256 +++++++++++++++--- 1 file changed, 219 insertions(+), 37 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 53fe00a14..940b063da 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -18,22 +18,123 @@ pragma solidity 0.8.17; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; -import "./../bridge/BitcoinTx.sol"; -import "./../bridge/Bridge.sol"; -import "./../bridge/Deposit.sol"; -import "./../vault/TBTCVault.sol"; -import "./../vault/TBTCOptimisticMinting.sol"; - -// TODO: Document the contract. +import "../bridge/BitcoinTx.sol"; +import "../bridge/Deposit.sol"; +import "../vault/TBTCOptimisticMinting.sol"; + +/// @notice Interface of the Bridge contract. +/// @dev See bridge/Bridge.sol +interface IBridge { + /// @dev See {Bridge#revealDepositWithExtraData} + function revealDepositWithExtraData( + BitcoinTx.Info calldata fundingTx, + Deposit.DepositRevealInfo calldata reveal, + bytes32 extraData + ) external; + + /// @dev See {Bridge#deposits} + function deposits(uint256 depositKey) + external + view + returns (Deposit.DepositRequest memory); + + /// @dev See {Bridge#depositParameters} + function depositParameters() + external + view + returns ( + uint64 depositDustThreshold, + uint64 depositTreasuryFeeDivisor, + uint64 depositTxMaxFee, + uint32 depositRevealAheadPeriod + ); +} + +/// @notice Interface of the TBTCVault contract. +/// @dev See vault/TBTCVault.sol +interface ITBTCVault { + /// @dev See {TBTCVault#optimisticMintingRequests} + function optimisticMintingRequests(uint256 depositKey) + external + returns (uint64 requestedAt, uint64 finalizedAt); + + /// @dev See {TBTCVault#optimisticMintingFeeDivisor} + function optimisticMintingFeeDivisor() external view returns (uint32); +} + +/// @title Abstract TBTCDepositorProxy contract. +/// @notice This abstract contract is meant to facilitate integration of protocols +/// aiming to use tBTC as an underlying Bitcoin bridge. +/// +/// Such an integrator is supposed to: +/// - Create a child contract inheriting from this abstract contract +/// - Implement the `onDepositFinalized`abstract function +/// - Call the `__TBTCDepositorProxy_initialize` initializer function +/// - Use the `initializeDeposit` and `finalizeDeposit` as part of their +/// business logic in order to initialize and finalize deposits. +/// +/// @dev Example usage: +/// ``` +/// // Example upgradeable integrator contract. +/// contract ExampleTBTCIntegrator is TBTCDepositorProxy { +/// /// @custom:oz-upgrades-unsafe-allow constructor +/// constructor() { +/// // Prevents the contract from being initialized again. +/// _disableInitializers(); +/// } +/// +/// function initialize( +/// address _bridge, +/// address _tbtcVault +/// ) external initializer { +/// __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); +/// } +/// +/// function startProcess( +/// BitcoinTx.Info calldata fundingTx, +/// Deposit.DepositRevealInfo calldata reveal +/// ) external { +/// // Embed necessary context as extra data. +/// bytes32 extraData = ...; +/// +/// uint256 depositKey = initializeDeposit( +/// fundingTx, +/// reveal, +/// extraData +/// ); +/// +/// // Use the depositKey to track the process. +/// } +/// +/// function finalizeProcess(uint256 depositKey) external { +/// // Finalize the deposit. This call will invoke the +/// // `onDepositFinalized` function. +/// finalizeDeposit(depositKey); +/// } +/// +/// function onDepositFinalized( +/// uint256 depositKey, +/// uint256 tbtcAmount, +/// bytes32 extraData +/// ) internal override { +/// // Do something with the minted TBTC using context +/// // embedded in the extraData. +/// } +/// } abstract contract TBTCDepositorProxy is Initializable { using BTCUtils for bytes; /// @notice Multiplier to convert satoshi to TBTC token units. uint256 public constant SATOSHI_MULTIPLIER = 10**10; - Bridge public bridge; - TBTCVault public tbtcVault; - + /// @notice Bridge contract address. + IBridge public bridge; + /// @notice TBTCVault contract address. + ITBTCVault public tbtcVault; + /// @notice Mapping holding information about pending deposits that have + /// been initialized but not finalized yet. If the deposit is not + /// in this mapping it means it has already been finalized or it + /// has not been initialized yet. mapping(uint256 => bool) public pendingDeposits; // Reserved storage space that allows adding more variables without affecting @@ -44,10 +145,7 @@ abstract contract TBTCDepositorProxy is Initializable { // slither-disable-next-line unused-state uint256[47] private __gap; - event DepositInitialized( - uint256 indexed depositKey, - uint32 initializedAt - ); + event DepositInitialized(uint256 indexed depositKey, uint32 initializedAt); event DepositFinalized( uint256 indexed depositKey, @@ -55,6 +153,8 @@ abstract contract TBTCDepositorProxy is Initializable { uint32 finalizedAt ); + /// @notice Initializes the contract. MUST BE CALLED from the child + /// contract initializer. function __TBTCDepositorProxy_initialize( address _bridge, address _tbtcVault @@ -62,10 +162,22 @@ abstract contract TBTCDepositorProxy is Initializable { require(_bridge != address(0), "Bridge address cannot be zero"); require(_tbtcVault != address(0), "TBTCVault address cannot be zero"); - bridge = Bridge(_bridge); - tbtcVault = TBTCVault(_tbtcVault); + bridge = IBridge(_bridge); + tbtcVault = ITBTCVault(_tbtcVault); } + /// @notice Initializes a deposit by revealing it to the Bridge. + /// @param fundingTx Bitcoin funding transaction data, see `BitcoinTx.Info`. + /// @param reveal Deposit reveal data, see `Deposit.DepositRevealInfo` struct. + /// @param extraData 32-byte deposit extra data. + /// @return depositKey Deposit key computed as + /// `keccak256(fundingTxHash | reveal.fundingOutputIndex)`. This + /// key can be used to refer to the deposit in the Bridge and + /// TBTCVault contracts. + /// @dev Requirements: + /// - The revealed vault address must match the TBTCVault address, + /// - All requirements from {Bridge#revealDepositWithExtraData} + /// function must be met. function initializeDeposit( BitcoinTx.Info calldata fundingTx, Deposit.DepositRevealInfo calldata reveal, @@ -95,15 +207,27 @@ abstract contract TBTCDepositorProxy is Initializable { return depositKey; } + /// @notice Finalizes a deposit by calculating the amount of TBTC minted + /// for the deposit and calling the `onDepositFinalized` callback + /// function. + /// @param depositKey Deposit key identifying the deposit. + /// @dev Requirements: + /// - The deposit must be initialized but not finalized + /// (in the context of this contract) yet. + /// - The deposit must be finalized on the Bridge side. That means the + /// deposit must be either swept or optimistically minted. function finalizeDeposit(uint256 depositKey) internal { require(pendingDeposits[depositKey], "Deposit not initialized"); Deposit.DepositRequest memory deposit = bridge.deposits(depositKey); + (, uint64 finalizedAt) = tbtcVault.optimisticMintingRequests( + depositKey + ); - bool minted = deposit.sweptAt != 0 || - tbtcVault.optimisticMintingRequests(depositKey).finalizedAt != 0; - - require(minted, "Deposit not finalized by the bridge"); + require( + deposit.sweptAt != 0 || finalizedAt != 0, + "Deposit not finalized by the bridge" + ); // We can safely delete the deposit from the pending deposits mapping. // This deposit cannot be initialized again because the bridge does not @@ -127,49 +251,107 @@ abstract contract TBTCDepositorProxy is Initializable { onDepositFinalized(depositKey, tbtcAmount, deposit.extraData); } + /// @notice Calculates the amount of TBTC minted for the deposit. + /// @param depositAmountSat Deposit amount in satoshi (1e8 precision). + /// This is the actual amount deposited by the deposit creator, i.e. + /// the gross amount the Bridge's fees are cut from. + /// @param depositTreasuryFeeSat Deposit treasury fee in satoshi (1e8 precision). + /// This is an accurate value of the treasury fee that was actually + /// cut upon minting. + /// @return tbtcAmount Approximate amount of TBTC minted for the deposit. + /// @dev IMPORTANT NOTE: The tbtcAmount returned by this function may + /// not correspond to the actual amount of TBTC minted for the deposit. + /// The amount of TBTC minted for the deposit is calculated using an + /// accurate value of the treasury fee that was cut upon minting. + /// However, there is no possibility to do the same for the optimistic + /// minting fee and the Bitcoin transaction fee. To overcome this, + /// this contract just takes the current maximum values of both fees + /// upon deposit finalization. For the great majority of the deposits, + /// such an algorithm will return a tbtcAmount slightly lesser than the + /// actual amount of TBTC minted for the deposit. This will cause + /// some TBTC to be left in the contract and ensure there is enough + /// liquidity to finalize the deposit. However, in some rare cases, + /// where the actual values of those fee change between the deposit + /// minting and finalization, the tbtcAmount returned by this function + /// may be greater than the actual amount of TBTC minted for the deposit. + /// If this happens and the reserve coming from previous deposits + /// leftovers does not provide enough liquidity, the deposit will have + /// wait for finalization until the reserve is refilled by subsequent + /// deposits or a manual top-up. The integrator is responsible for + /// handling such cases. function calculateTbtcAmount( uint64 depositAmountSat, uint64 depositTreasuryFeeSat - ) internal virtual view returns (uint256) { + ) internal view virtual returns (uint256) { // Both deposit amount and treasury fee are in the 1e8 satoshi precision. // We need to convert them to the 1e18 TBTC precision. uint256 amountSubTreasury = (depositAmountSat - depositTreasuryFeeSat) * - SATOSHI_MULTIPLIER; + SATOSHI_MULTIPLIER; uint256 omFeeDivisor = tbtcVault.optimisticMintingFeeDivisor(); - uint256 omFee = omFeeDivisor > 0 ? (amountSubTreasury / omFeeDivisor) : 0; + uint256 omFee = omFeeDivisor > 0 + ? (amountSubTreasury / omFeeDivisor) + : 0; // The deposit transaction max fee is in the 1e8 satoshi precision. // We need to convert them to the 1e18 TBTC precision. - (,,uint64 depositTxMaxFee,) = bridge.depositParameters(); + (, , uint64 depositTxMaxFee, ) = bridge.depositParameters(); uint256 txMaxFee = depositTxMaxFee * SATOSHI_MULTIPLIER; return amountSubTreasury - omFee - txMaxFee; } + /// @notice Callback function called when a deposit is finalized. + /// @param depositKey Deposit key identifying the deposit. + /// @param tbtcAmount Approximate amount of TBTC minted for the deposit. + /// @param extraData 32-byte deposit extra data. + /// @dev This function is called by the `finalizeDeposit` function. + /// The integrator is supposed to implement this function according + /// to their business logic. + /// @dev IMPORTANT NOTE: The tbtcAmount passed to this function is an + /// approximation. See documentation of the `calculateTbtcAmount` + /// responsible for calculating this value for more details. function onDepositFinalized( uint256 depositKey, uint256 tbtcAmount, bytes32 extraData ) internal virtual; + /// @notice Calculates the deposit key for the given funding transaction + /// hash and funding output index. + /// @param fundingTxHash Funding transaction hash. + /// @param fundingOutputIndex Funding output index. + /// @return depositKey Deposit key computed as + /// `keccak256(fundingTxHash | reveal.fundingOutputIndex)`. This + /// key can be used to refer to the deposit in the Bridge and + /// TBTCVault contracts. function calculateDepositKey( bytes32 fundingTxHash, uint32 fundingOutputIndex ) internal pure returns (uint256) { - return uint256( - keccak256(abi.encodePacked(fundingTxHash, fundingOutputIndex)) - ); + return + uint256( + keccak256(abi.encodePacked(fundingTxHash, fundingOutputIndex)) + ); } - function calculateBitcoinTxHash( - BitcoinTx.Info calldata txInfo - ) internal pure returns (bytes32) { - return abi.encodePacked( - txInfo.version, - txInfo.inputVector, - txInfo.outputVector, - txInfo.locktime - ).hash256View(); + /// @notice Calculates the Bitcoin transaction hash for the given Bitcoin + /// transaction data. + /// @param txInfo Bitcoin transaction data, see `BitcoinTx.Info` struct. + /// @return txHash Bitcoin transaction hash. + function calculateBitcoinTxHash(BitcoinTx.Info calldata txInfo) + internal + view + returns (bytes32) + { + return + abi + .encodePacked( + txInfo.version, + txInfo.inputVector, + txInfo.outputVector, + txInfo.locktime + ) + .hash256View(); } -} \ No newline at end of file +} From 357c2027d8e8e7d7257fe5bdab4ef0a7a613cc7f Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 18:22:47 +0100 Subject: [PATCH 10/22] Reword the docstring of `calculateTbtcAmount` function --- .../contracts/integrator/TBTCDepositorProxy.sol | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 940b063da..e04a4ad5e 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -261,13 +261,12 @@ abstract contract TBTCDepositorProxy is Initializable { /// @return tbtcAmount Approximate amount of TBTC minted for the deposit. /// @dev IMPORTANT NOTE: The tbtcAmount returned by this function may /// not correspond to the actual amount of TBTC minted for the deposit. - /// The amount of TBTC minted for the deposit is calculated using an - /// accurate value of the treasury fee that was cut upon minting. - /// However, there is no possibility to do the same for the optimistic - /// minting fee and the Bitcoin transaction fee. To overcome this, - /// this contract just takes the current maximum values of both fees - /// upon deposit finalization. For the great majority of the deposits, - /// such an algorithm will return a tbtcAmount slightly lesser than the + /// Although the treasury fee cut upon minting is known precisely, + /// this is not the case for the optimistic minting fee and the Bitcoin + /// transaction fee. To overcome that problem, this function just takes + /// the current maximum values of both fees, at the moment of deposit + /// finalization. For the great majority of the deposits, such an + /// algorithm will return a tbtcAmount slightly lesser than the /// actual amount of TBTC minted for the deposit. This will cause /// some TBTC to be left in the contract and ensure there is enough /// liquidity to finalize the deposit. However, in some rare cases, From 0ed4d7fc0dfcac3441f0b35ce9d83907281c167d Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 18:26:04 +0100 Subject: [PATCH 11/22] Fix typos in the docstring of the `calculateTbtcAmount` function --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index e04a4ad5e..ab866c145 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -270,12 +270,12 @@ abstract contract TBTCDepositorProxy is Initializable { /// actual amount of TBTC minted for the deposit. This will cause /// some TBTC to be left in the contract and ensure there is enough /// liquidity to finalize the deposit. However, in some rare cases, - /// where the actual values of those fee change between the deposit + /// where the actual values of those fees change between the deposit /// minting and finalization, the tbtcAmount returned by this function /// may be greater than the actual amount of TBTC minted for the deposit. /// If this happens and the reserve coming from previous deposits /// leftovers does not provide enough liquidity, the deposit will have - /// wait for finalization until the reserve is refilled by subsequent + /// to wait for finalization until the reserve is refilled by subsequent /// deposits or a manual top-up. The integrator is responsible for /// handling such cases. function calculateTbtcAmount( From ff77f7dfdeb1827de8d8ba0bb84d5dbbbbc737c7 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 29 Jan 2024 18:37:58 +0100 Subject: [PATCH 12/22] Suppress Slither false positives --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index ab866c145..e8427ef1c 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -155,6 +155,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// @notice Initializes the contract. MUST BE CALLED from the child /// contract initializer. + // slither-disable-next-line dead-code function __TBTCDepositorProxy_initialize( address _bridge, address _tbtcVault @@ -178,6 +179,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// - The revealed vault address must match the TBTCVault address, /// - All requirements from {Bridge#revealDepositWithExtraData} /// function must be met. + // slither-disable-next-line dead-code function initializeDeposit( BitcoinTx.Info calldata fundingTx, Deposit.DepositRevealInfo calldata reveal, @@ -216,6 +218,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// (in the context of this contract) yet. /// - The deposit must be finalized on the Bridge side. That means the /// deposit must be either swept or optimistically minted. + // slither-disable-next-line dead-code function finalizeDeposit(uint256 depositKey) internal { require(pendingDeposits[depositKey], "Deposit not initialized"); @@ -234,6 +237,7 @@ abstract contract TBTCDepositorProxy is Initializable { // allow to reveal the same deposit twice. Deleting the deposit from // the mapping will also prevent the finalizeDeposit function from // being called again for the same deposit. + // slither-disable-next-line reentrancy-no-eth delete pendingDeposits[depositKey]; uint256 tbtcAmount = calculateTbtcAmount( @@ -241,6 +245,7 @@ abstract contract TBTCDepositorProxy is Initializable { deposit.treasuryFee ); + // slither-disable-next-line reentrancy-events emit DepositFinalized( depositKey, tbtcAmount, @@ -278,6 +283,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// to wait for finalization until the reserve is refilled by subsequent /// deposits or a manual top-up. The integrator is responsible for /// handling such cases. + // slither-disable-next-line dead-code function calculateTbtcAmount( uint64 depositAmountSat, uint64 depositTreasuryFeeSat @@ -310,6 +316,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// @dev IMPORTANT NOTE: The tbtcAmount passed to this function is an /// approximation. See documentation of the `calculateTbtcAmount` /// responsible for calculating this value for more details. + // slither-disable-next-line unimplemented-functions function onDepositFinalized( uint256 depositKey, uint256 tbtcAmount, @@ -324,6 +331,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// `keccak256(fundingTxHash | reveal.fundingOutputIndex)`. This /// key can be used to refer to the deposit in the Bridge and /// TBTCVault contracts. + // slither-disable-next-line dead-code function calculateDepositKey( bytes32 fundingTxHash, uint32 fundingOutputIndex @@ -338,6 +346,7 @@ abstract contract TBTCDepositorProxy is Initializable { /// transaction data. /// @param txInfo Bitcoin transaction data, see `BitcoinTx.Info` struct. /// @return txHash Bitcoin transaction hash. + // slither-disable-next-line dead-code function calculateBitcoinTxHash(BitcoinTx.Info calldata txInfo) internal view From b7ed96eae8cd29e9eb785e192a9ea0f5963f0f1b Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Tue, 30 Jan 2024 10:27:26 +0100 Subject: [PATCH 13/22] More flexible pragma --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index e8427ef1c..a302e6bb0 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -13,7 +13,7 @@ // ▐████▌ ▐████▌ // ▐████▌ ▐████▌ -pragma solidity 0.8.17; +pragma solidity ^0.8.0; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; From f8a0d8f4a3a9a75ac41c503f3535e52833d6e747 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Tue, 30 Jan 2024 14:57:14 +0100 Subject: [PATCH 14/22] Extract `IBridge` and `ITBTCVault` to separate files This should increase readability and make reusing of those interfaces easier. This is important as the `integrator` package will likely contain more contracts (e.g. another one for redemptions). --- solidity/contracts/integrator/IBridge.sol | 47 +++++++++++++++++++ solidity/contracts/integrator/ITBTCVault.sol | 28 +++++++++++ .../integrator/TBTCDepositorProxy.sol | 42 +---------------- 3 files changed, 77 insertions(+), 40 deletions(-) create mode 100644 solidity/contracts/integrator/IBridge.sol create mode 100644 solidity/contracts/integrator/ITBTCVault.sol diff --git a/solidity/contracts/integrator/IBridge.sol b/solidity/contracts/integrator/IBridge.sol new file mode 100644 index 000000000..8af1866a8 --- /dev/null +++ b/solidity/contracts/integrator/IBridge.sol @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: GPL-3.0-only + +// ██████████████ ▐████▌ ██████████████ +// ██████████████ ▐████▌ ██████████████ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ██████████████ ▐████▌ ██████████████ +// ██████████████ ▐████▌ ██████████████ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ + +pragma solidity ^0.8.0; + +import "../bridge/BitcoinTx.sol"; +import "../bridge/Deposit.sol"; + +/// @notice Interface of the Bridge contract. +/// @dev See bridge/Bridge.sol +interface IBridge { + /// @dev See {Bridge#revealDepositWithExtraData} + function revealDepositWithExtraData( + BitcoinTx.Info calldata fundingTx, + Deposit.DepositRevealInfo calldata reveal, + bytes32 extraData + ) external; + + /// @dev See {Bridge#deposits} + function deposits(uint256 depositKey) + external + view + returns (Deposit.DepositRequest memory); + + /// @dev See {Bridge#depositParameters} + function depositParameters() + external + view + returns ( + uint64 depositDustThreshold, + uint64 depositTreasuryFeeDivisor, + uint64 depositTxMaxFee, + uint32 depositRevealAheadPeriod + ); +} diff --git a/solidity/contracts/integrator/ITBTCVault.sol b/solidity/contracts/integrator/ITBTCVault.sol new file mode 100644 index 000000000..a9f665637 --- /dev/null +++ b/solidity/contracts/integrator/ITBTCVault.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-3.0-only + +// ██████████████ ▐████▌ ██████████████ +// ██████████████ ▐████▌ ██████████████ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ██████████████ ▐████▌ ██████████████ +// ██████████████ ▐████▌ ██████████████ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ +// ▐████▌ ▐████▌ + +pragma solidity ^0.8.0; + +/// @notice Interface of the TBTCVault contract. +/// @dev See vault/TBTCVault.sol +interface ITBTCVault { + /// @dev See {TBTCVault#optimisticMintingRequests} + function optimisticMintingRequests(uint256 depositKey) + external + returns (uint64 requestedAt, uint64 finalizedAt); + + /// @dev See {TBTCVault#optimisticMintingFeeDivisor} + function optimisticMintingFeeDivisor() external view returns (uint32); +} diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index a302e6bb0..8ac00e927 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -20,47 +20,9 @@ import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Ini import "../bridge/BitcoinTx.sol"; import "../bridge/Deposit.sol"; -import "../vault/TBTCOptimisticMinting.sol"; -/// @notice Interface of the Bridge contract. -/// @dev See bridge/Bridge.sol -interface IBridge { - /// @dev See {Bridge#revealDepositWithExtraData} - function revealDepositWithExtraData( - BitcoinTx.Info calldata fundingTx, - Deposit.DepositRevealInfo calldata reveal, - bytes32 extraData - ) external; - - /// @dev See {Bridge#deposits} - function deposits(uint256 depositKey) - external - view - returns (Deposit.DepositRequest memory); - - /// @dev See {Bridge#depositParameters} - function depositParameters() - external - view - returns ( - uint64 depositDustThreshold, - uint64 depositTreasuryFeeDivisor, - uint64 depositTxMaxFee, - uint32 depositRevealAheadPeriod - ); -} - -/// @notice Interface of the TBTCVault contract. -/// @dev See vault/TBTCVault.sol -interface ITBTCVault { - /// @dev See {TBTCVault#optimisticMintingRequests} - function optimisticMintingRequests(uint256 depositKey) - external - returns (uint64 requestedAt, uint64 finalizedAt); - - /// @dev See {TBTCVault#optimisticMintingFeeDivisor} - function optimisticMintingFeeDivisor() external view returns (uint32); -} +import "./IBridge.sol"; +import "./ITBTCVault.sol"; /// @title Abstract TBTCDepositorProxy contract. /// @notice This abstract contract is meant to facilitate integration of protocols From 289251777a632a79b74ea4f4085a6f27e4808d13 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Tue, 30 Jan 2024 15:25:12 +0100 Subject: [PATCH 15/22] Unit tests for `TBTCDepositorProxy` --- .../contracts/test/TestTBTCDepositorProxy.sol | 170 ++++++++++ .../integrator/TBTCDepositorProxy.test.ts | 312 ++++++++++++++++++ 2 files changed, 482 insertions(+) create mode 100644 solidity/contracts/test/TestTBTCDepositorProxy.sol create mode 100644 solidity/test/integrator/TBTCDepositorProxy.test.ts diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol new file mode 100644 index 000000000..1b69409f8 --- /dev/null +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: GPL-3.0-only + +pragma solidity ^0.8.0; + +import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; + +import {TBTCDepositorProxy} from "../integrator/TBTCDepositorProxy.sol"; +import {IBridge} from "../integrator/IBridge.sol"; +import {ITBTCVault} from "../integrator/ITBTCVault.sol"; + +import "../bridge/BitcoinTx.sol"; +import "../bridge/Deposit.sol"; + +contract TestTBTCDepositorProxy is TBTCDepositorProxy { + event OnDepositFinalizedCalled( + uint256 depositKey, + uint256 tbtcAmount, + bytes32 extraData + ); + + function initialize(address _bridge, address _tbtcVault) + external + initializer + { + __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); + } + + function initializeDepositPublic( + BitcoinTx.Info calldata fundingTx, + Deposit.DepositRevealInfo calldata reveal, + bytes32 extraData + ) external { + initializeDeposit(fundingTx, reveal, extraData); + } + + function finalizeDepositPublic(uint256 depositKey) external { + finalizeDeposit(depositKey); + } + + function onDepositFinalized( + uint256 depositKey, + uint256 tbtcAmount, + bytes32 extraData + ) internal override { + emit OnDepositFinalizedCalled(depositKey, tbtcAmount, extraData); + } +} + +contract MockBridge is IBridge { + using BTCUtils for bytes; + + mapping(uint256 => Deposit.DepositRequest) internal _deposits; + + uint64 internal _depositTxMaxFee = 1 * 1e7; // 0.1 BTC + + event DepositRevealed(uint256 depositKey); + + function revealDepositWithExtraData( + BitcoinTx.Info calldata fundingTx, + Deposit.DepositRevealInfo calldata reveal, + bytes32 extraData + ) external { + bytes32 fundingTxHash = abi + .encodePacked( + fundingTx.version, + fundingTx.inputVector, + fundingTx.outputVector, + fundingTx.locktime + ) + .hash256View(); + + uint256 depositKey = uint256( + keccak256( + abi.encodePacked(fundingTxHash, reveal.fundingOutputIndex) + ) + ); + + require( + _deposits[depositKey].revealedAt == 0, + "Deposit already revealed" + ); + + Deposit.DepositRequest memory request; + + request.depositor = msg.sender; + request.amount = uint64(10 * 1e8); // 10 BTC + /* 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.sweptAt = 0; + request.extraData = extraData; + + _deposits[depositKey] = request; + + emit DepositRevealed(depositKey); + } + + function sweepDeposit(uint256 depositKey) external { + require(_deposits[depositKey].revealedAt != 0, "Deposit not revealed"); + require(_deposits[depositKey].sweptAt == 0, "Deposit already swept"); + /* solhint-disable-next-line not-rely-on-time */ + _deposits[depositKey].sweptAt = uint32(block.timestamp); + } + + function deposits(uint256 depositKey) + external + view + returns (Deposit.DepositRequest memory) + { + return _deposits[depositKey]; + } + + function depositParameters() + external + view + returns ( + uint64 depositDustThreshold, + uint64 depositTreasuryFeeDivisor, + uint64 depositTxMaxFee, + uint32 depositRevealAheadPeriod + ) + { + depositDustThreshold = 0; + depositTreasuryFeeDivisor = 0; + depositTxMaxFee = _depositTxMaxFee; + depositRevealAheadPeriod = 0; + } +} + +contract MockTBTCVault is ITBTCVault { + struct Request { + uint64 requestedAt; + uint64 finalizedAt; + } + + mapping(uint256 => Request) internal _requests; + + uint32 public optimisticMintingFeeDivisor = 100; // 1% + + function optimisticMintingRequests(uint256 depositKey) + external + returns (uint64 requestedAt, uint64 finalizedAt) + { + Request memory request = _requests[depositKey]; + return (request.requestedAt, request.finalizedAt); + } + + function createOptimisticMintingRequest(uint256 depositKey) external { + require( + _requests[depositKey].requestedAt == 0, + "Request already exists" + ); + /* solhint-disable-next-line not-rely-on-time */ + _requests[depositKey].requestedAt = uint64(block.timestamp); + } + + function finalizeOptimisticMintingRequest(uint256 depositKey) external { + require( + _requests[depositKey].requestedAt != 0, + "Request does not exist" + ); + require( + _requests[depositKey].finalizedAt == 0, + "Request already finalized" + ); + /* solhint-disable-next-line not-rely-on-time */ + _requests[depositKey].finalizedAt = uint64(block.timestamp); + } +} diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/TBTCDepositorProxy.test.ts new file mode 100644 index 000000000..759f2117f --- /dev/null +++ b/solidity/test/integrator/TBTCDepositorProxy.test.ts @@ -0,0 +1,312 @@ +import { ethers, helpers } from "hardhat" +import { expect } from "chai" +import { BigNumber, ContractTransaction } from "ethers" +import type { + MockBridge, + MockTBTCVault, + TestTBTCDepositorProxy, +} from "../../typechain" +import { to1ePrecision } from "../helpers/contract-test-helpers" + +const { createSnapshot, restoreSnapshot } = helpers.snapshot +const { lastBlockTime } = helpers.time + +const loadFixture = (vault: string) => ({ + fundingTx: { + version: "0x01000000", + inputVector: + "0x018348cdeb551134fe1f19d378a8adec9b146671cb67b945b71bf56b20d" + + "c2b952f0100000000ffffffff", + outputVector: + "0x021027000000000000220020bfaeddba12b0de6feeb649af76376876bc1" + + "feb6c2248fbfef9293ba3ac51bb4a10d73b00000000001600147ac2d9378a" + + "1c47e589dfb8095ca95ed2140d2726", + locktime: "0x00000000", + }, + reveal: { + fundingOutputIndex: 0, + blindingFactor: "0xf9f0c90d00039523", + walletPubKeyHash: "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + refundPubKeyHash: "0x28e081f285138ccbe389c1eb8985716230129f89", + refundLocktime: "0x60bcea61", + vault, + }, + extraData: + "0xa9b38ea6435c8941d6eda6a46b68e3e2117196995bd154ab55196396b03d9bda", + expectedDepositKey: + "0xebff13c2304229ab4a97bfbfabeac82c9c0704e4aae2acf022252ac8dc1101d1", +}) + +describe("TBTCDepositorProxy", () => { + let bridge: MockBridge + let tbtcVault: MockTBTCVault + let depositorProxy: TestTBTCDepositorProxy + + let fixture + + before(async () => { + const MockBridge = await ethers.getContractFactory("MockBridge") + bridge = await MockBridge.deploy() + + const MockTBTCVault = await ethers.getContractFactory("MockTBTCVault") + tbtcVault = await MockTBTCVault.deploy() + + fixture = loadFixture(tbtcVault.address) + + const TestTBTCDepositorProxy = await ethers.getContractFactory( + "TestTBTCDepositorProxy" + ) + depositorProxy = await TestTBTCDepositorProxy.deploy() + await depositorProxy.initialize(bridge.address, tbtcVault.address) + }) + + describe("initializeDeposit", () => { + context("when revealed vault does not match", () => { + it("should revert", async () => { + // Load the fixture with a different vault address. + const { fundingTx, reveal, extraData } = loadFixture( + ethers.constants.AddressZero + ) + + await expect( + depositorProxy.initializeDepositPublic(fundingTx, reveal, extraData) + ).to.be.revertedWith("Vault address mismatch") + }) + }) + + context("when revealed vault matches", () => { + context("when deposit is rejected by the Bridge", () => { + before(async () => { + await createSnapshot() + + // Pre-reveal the deposit to cause a revert on the second attempt + // made by the TBTCDepositorProxy. + await bridge.revealDepositWithExtraData( + fixture.fundingTx, + fixture.reveal, + fixture.extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + depositorProxy.initializeDepositPublic( + fixture.fundingTx, + fixture.reveal, + fixture.extraData + ) + ).to.be.revertedWith("Deposit already revealed") + }) + }) + + context("when deposit is accepted by the Bridge", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + tx = await depositorProxy.initializeDepositPublic( + fixture.fundingTx, + fixture.reveal, + fixture.extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should reveal the deposit to the Bridge", async () => { + await expect(tx) + .to.emit(bridge, "DepositRevealed") + .withArgs(fixture.expectedDepositKey) + }) + + it("should store the deposit as pending", async () => { + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + expect( + await depositorProxy.pendingDeposits(fixture.expectedDepositKey) + ).to.be.true + }) + + it("should emit the DepositInitialized event", async () => { + await expect(tx) + .to.emit(depositorProxy, "DepositInitialized") + .withArgs(fixture.expectedDepositKey, await lastBlockTime()) + }) + }) + }) + }) + + describe("finalizeDeposit", () => { + context("when deposit is not initialized", () => { + it("should revert", async () => { + await expect( + depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + ).to.be.revertedWith("Deposit not initialized") + }) + }) + + context("when deposit is already finalized", () => { + before(async () => { + await createSnapshot() + + await depositorProxy.initializeDepositPublic( + fixture.fundingTx, + fixture.reveal, + fixture.extraData + ) + + await bridge.sweepDeposit(fixture.expectedDepositKey) + + await depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + ).to.be.revertedWith("Deposit not initialized") + }) + }) + + context("when deposit is initialized but not finalized yet", () => { + before(async () => { + await createSnapshot() + + await depositorProxy.initializeDepositPublic( + fixture.fundingTx, + fixture.reveal, + fixture.extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + context("when deposit is not finalized by the Bridge", () => { + it("should revert", async () => { + await expect( + depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + ).to.be.revertedWith("Deposit not finalized by the bridge") + }) + }) + + context("when deposit is finalized by the Bridge", () => { + // The expected tbtcAmount is calculated as follows: + // + // - Deposited amount = 10 BTC (hardcoded in MockBridge) + // - Treasury fee = 1 BTC (hardcoded in MockBridge) + // - Optimistic minting fee = 1% (hardcoded in MockTBTCVault) + // - Transaction max fee = 0.1 BTC (hardcoded 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() + + context("when the deposit is swept", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + await bridge.sweepDeposit(fixture.expectedDepositKey) + + tx = await depositorProxy.finalizeDepositPublic( + fixture.expectedDepositKey + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should remove the deposit from pending", async () => { + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + expect( + await depositorProxy.pendingDeposits(fixture.expectedDepositKey) + ).to.be.false + }) + + it("should emit the DepositFinalized event", async () => { + await expect(tx) + .to.emit(depositorProxy, "DepositFinalized") + .withArgs( + fixture.expectedDepositKey, + expectedTbtcAmount, + await lastBlockTime() + ) + }) + + it("should call onDepositFinalized with proper params", async () => { + await expect(tx) + .to.emit(depositorProxy, "OnDepositFinalizedCalled") + .withArgs( + fixture.expectedDepositKey, + expectedTbtcAmount, + fixture.extraData + ) + }) + }) + + context("when the deposit is optimistically minted", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + await tbtcVault.createOptimisticMintingRequest( + fixture.expectedDepositKey + ) + + await tbtcVault.finalizeOptimisticMintingRequest( + fixture.expectedDepositKey + ) + + tx = await depositorProxy.finalizeDepositPublic( + fixture.expectedDepositKey + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should remove the deposit from pending", async () => { + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + expect( + await depositorProxy.pendingDeposits(fixture.expectedDepositKey) + ).to.be.false + }) + + it("should emit the DepositFinalized event", async () => { + await expect(tx) + .to.emit(depositorProxy, "DepositFinalized") + .withArgs( + fixture.expectedDepositKey, + expectedTbtcAmount, + await lastBlockTime() + ) + }) + + it("should call onDepositFinalized with proper params", async () => { + await expect(tx) + .to.emit(depositorProxy, "OnDepositFinalizedCalled") + .withArgs( + fixture.expectedDepositKey, + expectedTbtcAmount, + fixture.extraData + ) + }) + }) + }) + }) + }) +}) From 2c3c3c6ba4effe79ef9b9bb1cd86c863e632a79d Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Tue, 30 Jan 2024 17:24:22 +0100 Subject: [PATCH 16/22] Detach `TBTCDepositorProxy` from OpenZeppelin Here we make the `TBTCDepositorProxy` contract independent of the OZ contracts. This way, `TBTCDepositorProxy` can be used by child contracts alongside both older OZ v4 contracts and recent OZ v5 contracts. For maximum flexibility, we are keep using the classic storage gap convention to make `TBTCDepositorProxy` safe for upgrades. This way, this contract can coexist with OZ v5 contracts which use the ERC-7201 convention. According to ERC-7201 abstract (https://eips.ethereum.org/EIPS/eip-7201#abstract), ERC-7201 namespaces are safe against collisions with the standard storage layout used by Solidity. Forcing `TBTCDepositorProxy` to use ERC-7201 is not an option because the OZ Upgrade Plugin raises an error for solc versions <0.8.20 (`TBTCDepositorProxy` supports all 0.8.x versions) --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 12 ++++++++---- solidity/contracts/test/TestTBTCDepositorProxy.sol | 5 +---- solidity/test/integrator/TBTCDepositorProxy.test.ts | 5 +++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 8ac00e927..9e96ee60c 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -16,7 +16,6 @@ pragma solidity ^0.8.0; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; -import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "../bridge/BitcoinTx.sol"; import "../bridge/Deposit.sol"; @@ -38,7 +37,7 @@ import "./ITBTCVault.sol"; /// @dev Example usage: /// ``` /// // Example upgradeable integrator contract. -/// contract ExampleTBTCIntegrator is TBTCDepositorProxy { +/// contract ExampleTBTCIntegrator is TBTCDepositorProxy, Initializable { /// /// @custom:oz-upgrades-unsafe-allow constructor /// constructor() { /// // Prevents the contract from being initialized again. @@ -83,7 +82,7 @@ import "./ITBTCVault.sol"; /// // embedded in the extraData. /// } /// } -abstract contract TBTCDepositorProxy is Initializable { +abstract contract TBTCDepositorProxy { using BTCUtils for bytes; /// @notice Multiplier to convert satoshi to TBTC token units. @@ -121,7 +120,12 @@ abstract contract TBTCDepositorProxy is Initializable { function __TBTCDepositorProxy_initialize( address _bridge, address _tbtcVault - ) internal onlyInitializing { + ) internal { + require( + address(bridge) == address(0) && address(tbtcVault) == address(0), + "TBTCDepositorProxy already initialized" + ); + require(_bridge != address(0), "Bridge address cannot be zero"); require(_tbtcVault != address(0), "TBTCVault address cannot be zero"); diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 1b69409f8..7b6b02083 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -18,10 +18,7 @@ contract TestTBTCDepositorProxy is TBTCDepositorProxy { bytes32 extraData ); - function initialize(address _bridge, address _tbtcVault) - external - initializer - { + function initialize(address _bridge, address _tbtcVault) external { __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); } diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/TBTCDepositorProxy.test.ts index 759f2117f..309b8b399 100644 --- a/solidity/test/integrator/TBTCDepositorProxy.test.ts +++ b/solidity/test/integrator/TBTCDepositorProxy.test.ts @@ -58,6 +58,11 @@ describe("TBTCDepositorProxy", () => { ) depositorProxy = await TestTBTCDepositorProxy.deploy() await depositorProxy.initialize(bridge.address, tbtcVault.address) + + // Assert that contract initializer works as expected. + await expect( + depositorProxy.initialize(bridge.address, tbtcVault.address) + ).to.be.revertedWith("TBTCDepositorProxy already initialized") }) describe("initializeDeposit", () => { From a31abf464d48fdcd806bedcc2ddc4cc5f516ae04 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Wed, 31 Jan 2024 16:33:21 +0100 Subject: [PATCH 17/22] Remove `onDepositFinalized` abstract function Here we remove the `onDepositFinalized` function and return `tbtcAmount` and `extraData` from `finalizeDeposit` in a direct way. This should make writing child contracts even easier. Moreover, we are prefixing all functions with `_` to explicitly denote that all inherited functions are `internal` and reduce problems around name clashes. --- .../integrator/TBTCDepositorProxy.sol | 61 ++++++------------- .../contracts/test/TestTBTCDepositorProxy.sol | 22 +++---- .../integrator/TBTCDepositorProxy.test.ts | 26 ++++---- 3 files changed, 39 insertions(+), 70 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 9e96ee60c..42b5be53a 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -29,9 +29,8 @@ import "./ITBTCVault.sol"; /// /// Such an integrator is supposed to: /// - Create a child contract inheriting from this abstract contract -/// - Implement the `onDepositFinalized`abstract function /// - Call the `__TBTCDepositorProxy_initialize` initializer function -/// - Use the `initializeDeposit` and `finalizeDeposit` as part of their +/// - Use the `_initializeDeposit` and `_finalizeDeposit` as part of their /// business logic in order to initialize and finalize deposits. /// /// @dev Example usage: @@ -58,7 +57,7 @@ import "./ITBTCVault.sol"; /// // Embed necessary context as extra data. /// bytes32 extraData = ...; /// -/// uint256 depositKey = initializeDeposit( +/// uint256 depositKey = _initializeDeposit( /// fundingTx, /// reveal, /// extraData @@ -68,16 +67,8 @@ import "./ITBTCVault.sol"; /// } /// /// function finalizeProcess(uint256 depositKey) external { -/// // Finalize the deposit. This call will invoke the -/// // `onDepositFinalized` function. -/// finalizeDeposit(depositKey); -/// } +/// (uint256 tbtcAmount, bytes32 extraData) = _finalizeDeposit(depositKey); /// -/// function onDepositFinalized( -/// uint256 depositKey, -/// uint256 tbtcAmount, -/// bytes32 extraData -/// ) internal override { /// // Do something with the minted TBTC using context /// // embedded in the extraData. /// } @@ -146,15 +137,15 @@ abstract contract TBTCDepositorProxy { /// - All requirements from {Bridge#revealDepositWithExtraData} /// function must be met. // slither-disable-next-line dead-code - function initializeDeposit( + function _initializeDeposit( BitcoinTx.Info calldata fundingTx, Deposit.DepositRevealInfo calldata reveal, bytes32 extraData ) internal returns (uint256) { require(reveal.vault == address(tbtcVault), "Vault address mismatch"); - uint256 depositKey = calculateDepositKey( - calculateBitcoinTxHash(fundingTx), + uint256 depositKey = _calculateDepositKey( + _calculateBitcoinTxHash(fundingTx), reveal.fundingOutputIndex ); @@ -179,13 +170,21 @@ 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 extraData 32-byte deposit extra data. /// @dev Requirements: /// - The deposit must be initialized but not finalized /// (in the context of this contract) yet. /// - The deposit must be finalized on the Bridge side. That means the /// deposit must be either swept or optimistically minted. + /// @dev IMPORTANT NOTE: The tbtcAmount returned by this function is an + /// approximation. See documentation of the `calculateTbtcAmount` + /// responsible for calculating this value for more details. // slither-disable-next-line dead-code - function finalizeDeposit(uint256 depositKey) internal { + function _finalizeDeposit(uint256 depositKey) + internal + returns (uint256 tbtcAmount, bytes32 extraData) + { require(pendingDeposits[depositKey], "Deposit not initialized"); Deposit.DepositRequest memory deposit = bridge.deposits(depositKey); @@ -206,10 +205,7 @@ abstract contract TBTCDepositorProxy { // slither-disable-next-line reentrancy-no-eth delete pendingDeposits[depositKey]; - uint256 tbtcAmount = calculateTbtcAmount( - deposit.amount, - deposit.treasuryFee - ); + tbtcAmount = _calculateTbtcAmount(deposit.amount, deposit.treasuryFee); // slither-disable-next-line reentrancy-events emit DepositFinalized( @@ -219,7 +215,7 @@ abstract contract TBTCDepositorProxy { uint32(block.timestamp) ); - onDepositFinalized(depositKey, tbtcAmount, deposit.extraData); + extraData = deposit.extraData; } /// @notice Calculates the amount of TBTC minted for the deposit. @@ -250,7 +246,7 @@ abstract contract TBTCDepositorProxy { /// deposits or a manual top-up. The integrator is responsible for /// handling such cases. // slither-disable-next-line dead-code - function calculateTbtcAmount( + function _calculateTbtcAmount( uint64 depositAmountSat, uint64 depositTreasuryFeeSat ) internal view virtual returns (uint256) { @@ -272,23 +268,6 @@ abstract contract TBTCDepositorProxy { return amountSubTreasury - omFee - txMaxFee; } - /// @notice Callback function called when a deposit is finalized. - /// @param depositKey Deposit key identifying the deposit. - /// @param tbtcAmount Approximate amount of TBTC minted for the deposit. - /// @param extraData 32-byte deposit extra data. - /// @dev This function is called by the `finalizeDeposit` function. - /// The integrator is supposed to implement this function according - /// to their business logic. - /// @dev IMPORTANT NOTE: The tbtcAmount passed to this function is an - /// approximation. See documentation of the `calculateTbtcAmount` - /// responsible for calculating this value for more details. - // slither-disable-next-line unimplemented-functions - function onDepositFinalized( - uint256 depositKey, - uint256 tbtcAmount, - bytes32 extraData - ) internal virtual; - /// @notice Calculates the deposit key for the given funding transaction /// hash and funding output index. /// @param fundingTxHash Funding transaction hash. @@ -298,7 +277,7 @@ abstract contract TBTCDepositorProxy { /// key can be used to refer to the deposit in the Bridge and /// TBTCVault contracts. // slither-disable-next-line dead-code - function calculateDepositKey( + function _calculateDepositKey( bytes32 fundingTxHash, uint32 fundingOutputIndex ) internal pure returns (uint256) { @@ -313,7 +292,7 @@ abstract contract TBTCDepositorProxy { /// @param txInfo Bitcoin transaction data, see `BitcoinTx.Info` struct. /// @return txHash Bitcoin transaction hash. // slither-disable-next-line dead-code - function calculateBitcoinTxHash(BitcoinTx.Info calldata txInfo) + function _calculateBitcoinTxHash(BitcoinTx.Info calldata txInfo) internal view returns (bytes32) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 7b6b02083..40a188405 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -12,11 +12,9 @@ import "../bridge/BitcoinTx.sol"; import "../bridge/Deposit.sol"; contract TestTBTCDepositorProxy is TBTCDepositorProxy { - event OnDepositFinalizedCalled( - uint256 depositKey, - uint256 tbtcAmount, - bytes32 extraData - ); + event InitializeDepositReturned(uint256 depositKey); + + event FinalizeDepositReturned(uint256 tbtcAmount, bytes32 extraData); function initialize(address _bridge, address _tbtcVault) external { __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); @@ -27,19 +25,13 @@ contract TestTBTCDepositorProxy is TBTCDepositorProxy { Deposit.DepositRevealInfo calldata reveal, bytes32 extraData ) external { - initializeDeposit(fundingTx, reveal, extraData); + uint256 depositKey = _initializeDeposit(fundingTx, reveal, extraData); + emit InitializeDepositReturned(depositKey); } function finalizeDepositPublic(uint256 depositKey) external { - finalizeDeposit(depositKey); - } - - function onDepositFinalized( - uint256 depositKey, - uint256 tbtcAmount, - bytes32 extraData - ) internal override { - emit OnDepositFinalizedCalled(depositKey, tbtcAmount, extraData); + (uint256 tbtcAmount, bytes32 extraData) = _finalizeDeposit(depositKey); + emit FinalizeDepositReturned(tbtcAmount, extraData); } } diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/TBTCDepositorProxy.test.ts index 309b8b399..290a0ebad 100644 --- a/solidity/test/integrator/TBTCDepositorProxy.test.ts +++ b/solidity/test/integrator/TBTCDepositorProxy.test.ts @@ -143,6 +143,12 @@ describe("TBTCDepositorProxy", () => { .to.emit(depositorProxy, "DepositInitialized") .withArgs(fixture.expectedDepositKey, await lastBlockTime()) }) + + it("should return proper values", async () => { + await expect(tx) + .to.emit(depositorProxy, "InitializeDepositReturned") + .withArgs(fixture.expectedDepositKey) + }) }) }) }) @@ -250,14 +256,10 @@ describe("TBTCDepositorProxy", () => { ) }) - it("should call onDepositFinalized with proper params", async () => { + it("should return proper values", async () => { await expect(tx) - .to.emit(depositorProxy, "OnDepositFinalizedCalled") - .withArgs( - fixture.expectedDepositKey, - expectedTbtcAmount, - fixture.extraData - ) + .to.emit(depositorProxy, "FinalizeDepositReturned") + .withArgs(expectedTbtcAmount, fixture.extraData) }) }) @@ -301,14 +303,10 @@ describe("TBTCDepositorProxy", () => { ) }) - it("should call onDepositFinalized with proper params", async () => { + it("should return proper values", async () => { await expect(tx) - .to.emit(depositorProxy, "OnDepositFinalizedCalled") - .withArgs( - fixture.expectedDepositKey, - expectedTbtcAmount, - fixture.extraData - ) + .to.emit(depositorProxy, "FinalizeDepositReturned") + .withArgs(expectedTbtcAmount, fixture.extraData) }) }) }) From 9efd0d0cc7ce4462b761886cc20fc8b31cc951f8 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Wed, 31 Jan 2024 18:05:52 +0100 Subject: [PATCH 18/22] Unit tests for `_calculateTbtcAmount` function --- .../contracts/test/TestTBTCDepositorProxy.sol | 15 ++ .../integrator/TBTCDepositorProxy.test.ts | 169 +++++++++++++++++- 2 files changed, 179 insertions(+), 5 deletions(-) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index 40a188405..eb91477b4 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -33,6 +33,13 @@ contract TestTBTCDepositorProxy is TBTCDepositorProxy { (uint256 tbtcAmount, bytes32 extraData) = _finalizeDeposit(depositKey); emit FinalizeDepositReturned(tbtcAmount, extraData); } + + function calculateTbtcAmountPublic( + uint64 depositAmountSat, + uint64 depositTreasuryFeeSat + ) external view returns (uint256) { + return _calculateTbtcAmount(depositAmountSat, depositTreasuryFeeSat); + } } contract MockBridge is IBridge { @@ -115,6 +122,10 @@ contract MockBridge is IBridge { depositTxMaxFee = _depositTxMaxFee; depositRevealAheadPeriod = 0; } + + function setDepositTxMaxFee(uint64 value) external { + _depositTxMaxFee = value; + } } contract MockTBTCVault is ITBTCVault { @@ -156,4 +167,8 @@ contract MockTBTCVault is ITBTCVault { /* solhint-disable-next-line not-rely-on-time */ _requests[depositKey].finalizedAt = uint64(block.timestamp); } + + function setOptimisticMintingFeeDivisor(uint32 value) external { + optimisticMintingFeeDivisor = value; + } } diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/TBTCDepositorProxy.test.ts index 290a0ebad..f2c39d457 100644 --- a/solidity/test/integrator/TBTCDepositorProxy.test.ts +++ b/solidity/test/integrator/TBTCDepositorProxy.test.ts @@ -65,7 +65,7 @@ describe("TBTCDepositorProxy", () => { ).to.be.revertedWith("TBTCDepositorProxy already initialized") }) - describe("initializeDeposit", () => { + describe("_initializeDeposit", () => { context("when revealed vault does not match", () => { it("should revert", async () => { // Load the fixture with a different vault address. @@ -153,7 +153,7 @@ describe("TBTCDepositorProxy", () => { }) }) - describe("finalizeDeposit", () => { + describe("_finalizeDeposit", () => { context("when deposit is not initialized", () => { it("should revert", async () => { await expect( @@ -214,10 +214,10 @@ describe("TBTCDepositorProxy", () => { context("when deposit is finalized by the Bridge", () => { // The expected tbtcAmount is calculated as follows: // - // - Deposited amount = 10 BTC (hardcoded in MockBridge) + // - Deposit amount = 10 BTC (hardcoded in MockBridge) // - Treasury fee = 1 BTC (hardcoded in MockBridge) - // - Optimistic minting fee = 1% (hardcoded in MockTBTCVault) - // - Transaction max fee = 0.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) // // ((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() @@ -312,4 +312,163 @@ describe("TBTCDepositorProxy", () => { }) }) }) + + describe("_calculateTbtcAmount", () => { + context("when all fees are non-zero", () => { + it("should return the correct amount", async () => { + const depositAmount = to1ePrecision(10, 8) // 10 BTC + const treasuryFee = to1ePrecision(1, 8) // 1 BTC + + // The expected tbtcAmount is calculated as follows: + // + // - 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) + // + // ((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) + + expect( + await depositorProxy.calculateTbtcAmountPublic( + depositAmount, + treasuryFee + ) + ).to.equal(expectedTbtcAmount) + }) + }) + + context("when all fees are zero", () => { + before(async () => { + await createSnapshot() + + // Set the transaction max fee to 0. + await bridge.setDepositTxMaxFee(0) + // Set the optimistic minting fee to 0%. + await tbtcVault.setOptimisticMintingFeeDivisor(0) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should return the correct amount", async () => { + const depositAmount = to1ePrecision(10, 8) // 10 BTC + const treasuryFee = BigNumber.from(0) + + // The expected tbtcAmount is calculated as follows: + // + // - Deposit amount = 10 BTC + // - Treasury fee = 0 BTC + // - Optimistic minting fee = 0% (set in MockTBTCVault) + // - Transaction max fee = 0 BTC (set in MockBridge) + // + // ((10 BTC - 0 BTC) * 1) - 0 BTC = 10 BTC = 10 * 1e18 TBTC + const expectedTbtcAmount = to1ePrecision(10, 18) + + expect( + await depositorProxy.calculateTbtcAmountPublic( + depositAmount, + treasuryFee + ) + ).to.equal(expectedTbtcAmount) + }) + }) + + context("when one of the fees is zero", () => { + context("when treasury fee is zero", () => { + it("should return the correct amount", async () => { + const depositAmount = to1ePrecision(10, 8) // 10 BTC + const treasuryFee = BigNumber.from(0) + + // The expected tbtcAmount is calculated as follows: + // + // - 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) + // + // ((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) + + expect( + await depositorProxy.calculateTbtcAmountPublic( + depositAmount, + treasuryFee + ) + ).to.equal(expectedTbtcAmount) + }) + }) + + context("when optimistic minting fee is zero", () => { + before(async () => { + await createSnapshot() + + // Set the optimistic minting fee to 0%. + await tbtcVault.setOptimisticMintingFeeDivisor(0) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should return the correct amount", async () => { + const depositAmount = to1ePrecision(10, 8) // 10 BTC + const treasuryFee = to1ePrecision(1, 8) // 1 BTC + + // The expected tbtcAmount is calculated as follows: + // + // - 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) + // + // ((10 BTC - 1 BTC) * 1) - 0.1 BTC = 8.9 BTC = 8.9 * 1e8 sat = 8.9 * 1e18 TBTC + const expectedTbtcAmount = to1ePrecision(89, 17) + + expect( + await depositorProxy.calculateTbtcAmountPublic( + depositAmount, + treasuryFee + ) + ).to.equal(expectedTbtcAmount) + }) + }) + + context("when transaction max fee is zero", () => { + before(async () => { + await createSnapshot() + + // Set the transaction max fee to 0. + await bridge.setDepositTxMaxFee(0) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should return the correct amount", async () => { + const depositAmount = to1ePrecision(10, 8) // 10 BTC + const treasuryFee = to1ePrecision(1, 8) // 1 BTC + + // The expected tbtcAmount is calculated as follows: + // + // - Deposit amount = 10 BTC + // - Treasury fee = 1 BTC + // - Optimistic minting fee = 1% (default value used in MockTBTCVault) + // - Transaction max fee = 0 BTC (set in MockBridge) + // + // ((10 BTC - 1 BTC) * 0.99) - 0 BTC = 8.91 BTC = 8.91 * 1e8 sat = 8.91 * 1e18 TBTC + const expectedTbtcAmount = to1ePrecision(891, 16) + + expect( + await depositorProxy.calculateTbtcAmountPublic( + depositAmount, + treasuryFee + ) + ).to.equal(expectedTbtcAmount) + }) + }) + }) + }) }) From 609b6fedc9444688092f310c38c0e03758bfdbfb Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Wed, 31 Jan 2024 18:28:17 +0100 Subject: [PATCH 19/22] Mirror `Bridge` types in the `integrator` subpackage This way, the `integrator` subpackage does not need to import anything from the `bridge` subpackage and explicitly depend on it. This simplifies the dependency graph for integrators. --- solidity/contracts/integrator/IBridge.sol | 43 ++++++++++++++++--- .../integrator/TBTCDepositorProxy.sol | 23 +++++----- .../contracts/test/TestTBTCDepositorProxy.sol | 23 +++++----- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/solidity/contracts/integrator/IBridge.sol b/solidity/contracts/integrator/IBridge.sol index 8af1866a8..ee3c673ef 100644 --- a/solidity/contracts/integrator/IBridge.sol +++ b/solidity/contracts/integrator/IBridge.sol @@ -15,16 +15,49 @@ pragma solidity ^0.8.0; -import "../bridge/BitcoinTx.sol"; -import "../bridge/Deposit.sol"; +/// @notice Namespace which groups all types relevant to the IBridge interface. +/// @dev This is a mirror of the real types used in the Bridge contract. +/// This way, the `integrator` subpackage does not need to import +/// anything from the `bridge` subpackage and explicitly depend on it. +/// This simplifies the dependency graph for integrators. +library IBridgeTypes { + /// @dev See bridge/BitcoinTx.sol#Info + struct BitcoinTxInfo { + bytes4 version; + bytes inputVector; + bytes outputVector; + bytes4 locktime; + } + + /// @dev See bridge/Deposit.sol#DepositRevealInfo + struct DepositRevealInfo { + uint32 fundingOutputIndex; + bytes8 blindingFactor; + bytes20 walletPubKeyHash; + bytes20 refundPubKeyHash; + bytes4 refundLocktime; + address vault; + } + + /// @dev See bridge/Deposit.sol#DepositRequest + struct DepositRequest { + address depositor; + uint64 amount; + uint32 revealedAt; + address vault; + uint64 treasuryFee; + uint32 sweptAt; + bytes32 extraData; + } +} /// @notice Interface of the Bridge contract. /// @dev See bridge/Bridge.sol interface IBridge { /// @dev See {Bridge#revealDepositWithExtraData} function revealDepositWithExtraData( - BitcoinTx.Info calldata fundingTx, - Deposit.DepositRevealInfo calldata reveal, + IBridgeTypes.BitcoinTxInfo calldata fundingTx, + IBridgeTypes.DepositRevealInfo calldata reveal, bytes32 extraData ) external; @@ -32,7 +65,7 @@ interface IBridge { function deposits(uint256 depositKey) external view - returns (Deposit.DepositRequest memory); + returns (IBridgeTypes.DepositRequest memory); /// @dev See {Bridge#depositParameters} function depositParameters() diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 42b5be53a..ecb75de1b 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -17,9 +17,6 @@ pragma solidity ^0.8.0; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; -import "../bridge/BitcoinTx.sol"; -import "../bridge/Deposit.sol"; - import "./IBridge.sol"; import "./ITBTCVault.sol"; @@ -51,8 +48,8 @@ import "./ITBTCVault.sol"; /// } /// /// function startProcess( -/// BitcoinTx.Info calldata fundingTx, -/// Deposit.DepositRevealInfo calldata reveal +/// IBridgeTypes.BitcoinTxInfo calldata fundingTx, +/// IBridgeTypes.DepositRevealInfo calldata reveal /// ) external { /// // Embed necessary context as extra data. /// bytes32 extraData = ...; @@ -125,8 +122,8 @@ abstract contract TBTCDepositorProxy { } /// @notice Initializes a deposit by revealing it to the Bridge. - /// @param fundingTx Bitcoin funding transaction data, see `BitcoinTx.Info`. - /// @param reveal Deposit reveal data, see `Deposit.DepositRevealInfo` struct. + /// @param fundingTx Bitcoin funding transaction data, see `IBridgeTypes.BitcoinTxInfo`. + /// @param reveal Deposit reveal data, see `IBridgeTypes.DepositRevealInfo` struct. /// @param extraData 32-byte deposit extra data. /// @return depositKey Deposit key computed as /// `keccak256(fundingTxHash | reveal.fundingOutputIndex)`. This @@ -138,8 +135,8 @@ abstract contract TBTCDepositorProxy { /// function must be met. // slither-disable-next-line dead-code function _initializeDeposit( - BitcoinTx.Info calldata fundingTx, - Deposit.DepositRevealInfo calldata reveal, + IBridgeTypes.BitcoinTxInfo calldata fundingTx, + IBridgeTypes.DepositRevealInfo calldata reveal, bytes32 extraData ) internal returns (uint256) { require(reveal.vault == address(tbtcVault), "Vault address mismatch"); @@ -187,7 +184,9 @@ abstract contract TBTCDepositorProxy { { require(pendingDeposits[depositKey], "Deposit not initialized"); - Deposit.DepositRequest memory deposit = bridge.deposits(depositKey); + IBridgeTypes.DepositRequest memory deposit = bridge.deposits( + depositKey + ); (, uint64 finalizedAt) = tbtcVault.optimisticMintingRequests( depositKey ); @@ -289,10 +288,10 @@ abstract contract TBTCDepositorProxy { /// @notice Calculates the Bitcoin transaction hash for the given Bitcoin /// transaction data. - /// @param txInfo Bitcoin transaction data, see `BitcoinTx.Info` struct. + /// @param txInfo Bitcoin transaction data, see `IBridgeTypes.BitcoinTxInfo` struct. /// @return txHash Bitcoin transaction hash. // slither-disable-next-line dead-code - function _calculateBitcoinTxHash(BitcoinTx.Info calldata txInfo) + function _calculateBitcoinTxHash(IBridgeTypes.BitcoinTxInfo calldata txInfo) internal view returns (bytes32) diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositorProxy.sol index eb91477b4..dd611503c 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositorProxy.sol @@ -4,12 +4,9 @@ pragma solidity ^0.8.0; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; -import {TBTCDepositorProxy} from "../integrator/TBTCDepositorProxy.sol"; -import {IBridge} from "../integrator/IBridge.sol"; -import {ITBTCVault} from "../integrator/ITBTCVault.sol"; - -import "../bridge/BitcoinTx.sol"; -import "../bridge/Deposit.sol"; +import "../integrator/TBTCDepositorProxy.sol"; +import "../integrator/IBridge.sol"; +import "../integrator/ITBTCVault.sol"; contract TestTBTCDepositorProxy is TBTCDepositorProxy { event InitializeDepositReturned(uint256 depositKey); @@ -21,8 +18,8 @@ contract TestTBTCDepositorProxy is TBTCDepositorProxy { } function initializeDepositPublic( - BitcoinTx.Info calldata fundingTx, - Deposit.DepositRevealInfo calldata reveal, + IBridgeTypes.BitcoinTxInfo calldata fundingTx, + IBridgeTypes.DepositRevealInfo calldata reveal, bytes32 extraData ) external { uint256 depositKey = _initializeDeposit(fundingTx, reveal, extraData); @@ -45,15 +42,15 @@ contract TestTBTCDepositorProxy is TBTCDepositorProxy { contract MockBridge is IBridge { using BTCUtils for bytes; - mapping(uint256 => Deposit.DepositRequest) internal _deposits; + mapping(uint256 => IBridgeTypes.DepositRequest) internal _deposits; uint64 internal _depositTxMaxFee = 1 * 1e7; // 0.1 BTC event DepositRevealed(uint256 depositKey); function revealDepositWithExtraData( - BitcoinTx.Info calldata fundingTx, - Deposit.DepositRevealInfo calldata reveal, + IBridgeTypes.BitcoinTxInfo calldata fundingTx, + IBridgeTypes.DepositRevealInfo calldata reveal, bytes32 extraData ) external { bytes32 fundingTxHash = abi @@ -76,7 +73,7 @@ contract MockBridge is IBridge { "Deposit already revealed" ); - Deposit.DepositRequest memory request; + IBridgeTypes.DepositRequest memory request; request.depositor = msg.sender; request.amount = uint64(10 * 1e8); // 10 BTC @@ -102,7 +99,7 @@ contract MockBridge is IBridge { function deposits(uint256 depositKey) external view - returns (Deposit.DepositRequest memory) + returns (IBridgeTypes.DepositRequest memory) { return _deposits[depositKey]; } From db5990507c4890bee79b1b3f7743e4e6ae90df5d Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 1 Feb 2024 17:42:40 +0100 Subject: [PATCH 20/22] Update docstring of `_finalizeDeposit` --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index ecb75de1b..2aa3ee847 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -164,8 +164,7 @@ abstract contract TBTCDepositorProxy { } /// @notice Finalizes a deposit by calculating the amount of TBTC minted - /// for the deposit and calling the `onDepositFinalized` callback - /// function. + /// for the deposit /// @param depositKey Deposit key identifying the deposit. /// @return tbtcAmount Approximate amount of TBTC minted for the deposit. /// @return extraData 32-byte deposit extra data. From a6f4a92771734466c18989c498754e3753e40651 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 1 Feb 2024 17:44:57 +0100 Subject: [PATCH 21/22] Update docstring of `_calculateTbtcAmount` --- solidity/contracts/integrator/TBTCDepositorProxy.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/TBTCDepositorProxy.sol index 2aa3ee847..38fbfc768 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/TBTCDepositorProxy.sol @@ -229,7 +229,7 @@ abstract contract TBTCDepositorProxy { /// Although the treasury fee cut upon minting is known precisely, /// this is not the case for the optimistic minting fee and the Bitcoin /// transaction fee. To overcome that problem, this function just takes - /// the current maximum values of both fees, at the moment of deposit + /// the current maximum allowed values of both fees, at the moment of deposit /// finalization. For the great majority of the deposits, such an /// algorithm will return a tbtcAmount slightly lesser than the /// actual amount of TBTC minted for the deposit. This will cause From 68d02e86f84b3a86f336b3f66104c598fcf083cf Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 1 Feb 2024 18:19:41 +0100 Subject: [PATCH 22/22] Rename `TBTCDepositorProxy` to `AbstractTBTCDepositor` --- ...torProxy.sol => AbstractTBTCDepositor.sol} | 14 +-- ...positorProxy.sol => TestTBTCDepositor.sol} | 6 +- ....test.ts => AbstractTBTCDepositor.test.ts} | 85 +++++++++---------- 3 files changed, 48 insertions(+), 57 deletions(-) rename solidity/contracts/integrator/{TBTCDepositorProxy.sol => AbstractTBTCDepositor.sol} (96%) rename solidity/contracts/test/{TestTBTCDepositorProxy.sol => TestTBTCDepositor.sol} (96%) rename solidity/test/integrator/{TBTCDepositorProxy.test.ts => AbstractTBTCDepositor.test.ts} (84%) diff --git a/solidity/contracts/integrator/TBTCDepositorProxy.sol b/solidity/contracts/integrator/AbstractTBTCDepositor.sol similarity index 96% rename from solidity/contracts/integrator/TBTCDepositorProxy.sol rename to solidity/contracts/integrator/AbstractTBTCDepositor.sol index 38fbfc768..845582070 100644 --- a/solidity/contracts/integrator/TBTCDepositorProxy.sol +++ b/solidity/contracts/integrator/AbstractTBTCDepositor.sol @@ -20,20 +20,20 @@ import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; import "./IBridge.sol"; import "./ITBTCVault.sol"; -/// @title Abstract TBTCDepositorProxy contract. +/// @title Abstract AbstractTBTCDepositor contract. /// @notice This abstract contract is meant to facilitate integration of protocols /// aiming to use tBTC as an underlying Bitcoin bridge. /// /// Such an integrator is supposed to: /// - Create a child contract inheriting from this abstract contract -/// - Call the `__TBTCDepositorProxy_initialize` initializer function +/// - Call the `__AbstractTBTCDepositor_initialize` initializer function /// - Use the `_initializeDeposit` and `_finalizeDeposit` as part of their /// business logic in order to initialize and finalize deposits. /// /// @dev Example usage: /// ``` /// // Example upgradeable integrator contract. -/// contract ExampleTBTCIntegrator is TBTCDepositorProxy, Initializable { +/// contract ExampleTBTCIntegrator is AbstractTBTCDepositor, Initializable { /// /// @custom:oz-upgrades-unsafe-allow constructor /// constructor() { /// // Prevents the contract from being initialized again. @@ -44,7 +44,7 @@ import "./ITBTCVault.sol"; /// address _bridge, /// address _tbtcVault /// ) external initializer { -/// __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); +/// __AbstractTBTCDepositor_initialize(_bridge, _tbtcVault); /// } /// /// function startProcess( @@ -70,7 +70,7 @@ import "./ITBTCVault.sol"; /// // embedded in the extraData. /// } /// } -abstract contract TBTCDepositorProxy { +abstract contract AbstractTBTCDepositor { using BTCUtils for bytes; /// @notice Multiplier to convert satoshi to TBTC token units. @@ -105,13 +105,13 @@ abstract contract TBTCDepositorProxy { /// @notice Initializes the contract. MUST BE CALLED from the child /// contract initializer. // slither-disable-next-line dead-code - function __TBTCDepositorProxy_initialize( + function __AbstractTBTCDepositor_initialize( address _bridge, address _tbtcVault ) internal { require( address(bridge) == address(0) && address(tbtcVault) == address(0), - "TBTCDepositorProxy already initialized" + "AbstractTBTCDepositor already initialized" ); require(_bridge != address(0), "Bridge address cannot be zero"); diff --git a/solidity/contracts/test/TestTBTCDepositorProxy.sol b/solidity/contracts/test/TestTBTCDepositor.sol similarity index 96% rename from solidity/contracts/test/TestTBTCDepositorProxy.sol rename to solidity/contracts/test/TestTBTCDepositor.sol index dd611503c..60b58a569 100644 --- a/solidity/contracts/test/TestTBTCDepositorProxy.sol +++ b/solidity/contracts/test/TestTBTCDepositor.sol @@ -4,17 +4,17 @@ pragma solidity ^0.8.0; import {BTCUtils} from "@keep-network/bitcoin-spv-sol/contracts/BTCUtils.sol"; -import "../integrator/TBTCDepositorProxy.sol"; +import "../integrator/AbstractTBTCDepositor.sol"; import "../integrator/IBridge.sol"; import "../integrator/ITBTCVault.sol"; -contract TestTBTCDepositorProxy is TBTCDepositorProxy { +contract TestTBTCDepositor is AbstractTBTCDepositor { event InitializeDepositReturned(uint256 depositKey); event FinalizeDepositReturned(uint256 tbtcAmount, bytes32 extraData); function initialize(address _bridge, address _tbtcVault) external { - __TBTCDepositorProxy_initialize(_bridge, _tbtcVault); + __AbstractTBTCDepositor_initialize(_bridge, _tbtcVault); } function initializeDepositPublic( diff --git a/solidity/test/integrator/TBTCDepositorProxy.test.ts b/solidity/test/integrator/AbstractTBTCDepositor.test.ts similarity index 84% rename from solidity/test/integrator/TBTCDepositorProxy.test.ts rename to solidity/test/integrator/AbstractTBTCDepositor.test.ts index f2c39d457..7e4486097 100644 --- a/solidity/test/integrator/TBTCDepositorProxy.test.ts +++ b/solidity/test/integrator/AbstractTBTCDepositor.test.ts @@ -4,7 +4,7 @@ import { BigNumber, ContractTransaction } from "ethers" import type { MockBridge, MockTBTCVault, - TestTBTCDepositorProxy, + TestTBTCDepositor, } from "../../typechain" import { to1ePrecision } from "../helpers/contract-test-helpers" @@ -37,10 +37,10 @@ const loadFixture = (vault: string) => ({ "0xebff13c2304229ab4a97bfbfabeac82c9c0704e4aae2acf022252ac8dc1101d1", }) -describe("TBTCDepositorProxy", () => { +describe("AbstractTBTCDepositor", () => { let bridge: MockBridge let tbtcVault: MockTBTCVault - let depositorProxy: TestTBTCDepositorProxy + let depositor: TestTBTCDepositor let fixture @@ -53,16 +53,16 @@ describe("TBTCDepositorProxy", () => { fixture = loadFixture(tbtcVault.address) - const TestTBTCDepositorProxy = await ethers.getContractFactory( - "TestTBTCDepositorProxy" + const TestTBTCDepositor = await ethers.getContractFactory( + "TestTBTCDepositor" ) - depositorProxy = await TestTBTCDepositorProxy.deploy() - await depositorProxy.initialize(bridge.address, tbtcVault.address) + depositor = await TestTBTCDepositor.deploy() + await depositor.initialize(bridge.address, tbtcVault.address) // Assert that contract initializer works as expected. await expect( - depositorProxy.initialize(bridge.address, tbtcVault.address) - ).to.be.revertedWith("TBTCDepositorProxy already initialized") + depositor.initialize(bridge.address, tbtcVault.address) + ).to.be.revertedWith("AbstractTBTCDepositor already initialized") }) describe("_initializeDeposit", () => { @@ -74,7 +74,7 @@ describe("TBTCDepositorProxy", () => { ) await expect( - depositorProxy.initializeDepositPublic(fundingTx, reveal, extraData) + depositor.initializeDepositPublic(fundingTx, reveal, extraData) ).to.be.revertedWith("Vault address mismatch") }) }) @@ -85,7 +85,7 @@ describe("TBTCDepositorProxy", () => { await createSnapshot() // Pre-reveal the deposit to cause a revert on the second attempt - // made by the TBTCDepositorProxy. + // made by the AbstractTBTCDepositor. await bridge.revealDepositWithExtraData( fixture.fundingTx, fixture.reveal, @@ -99,7 +99,7 @@ describe("TBTCDepositorProxy", () => { it("should revert", async () => { await expect( - depositorProxy.initializeDepositPublic( + depositor.initializeDepositPublic( fixture.fundingTx, fixture.reveal, fixture.extraData @@ -114,7 +114,7 @@ describe("TBTCDepositorProxy", () => { before(async () => { await createSnapshot() - tx = await depositorProxy.initializeDepositPublic( + tx = await depositor.initializeDepositPublic( fixture.fundingTx, fixture.reveal, fixture.extraData @@ -133,20 +133,19 @@ describe("TBTCDepositorProxy", () => { it("should store the deposit as pending", async () => { // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect( - await depositorProxy.pendingDeposits(fixture.expectedDepositKey) - ).to.be.true + expect(await depositor.pendingDeposits(fixture.expectedDepositKey)).to + .be.true }) it("should emit the DepositInitialized event", async () => { await expect(tx) - .to.emit(depositorProxy, "DepositInitialized") + .to.emit(depositor, "DepositInitialized") .withArgs(fixture.expectedDepositKey, await lastBlockTime()) }) it("should return proper values", async () => { await expect(tx) - .to.emit(depositorProxy, "InitializeDepositReturned") + .to.emit(depositor, "InitializeDepositReturned") .withArgs(fixture.expectedDepositKey) }) }) @@ -157,7 +156,7 @@ describe("TBTCDepositorProxy", () => { context("when deposit is not initialized", () => { it("should revert", async () => { await expect( - depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + depositor.finalizeDepositPublic(fixture.expectedDepositKey) ).to.be.revertedWith("Deposit not initialized") }) }) @@ -166,7 +165,7 @@ describe("TBTCDepositorProxy", () => { before(async () => { await createSnapshot() - await depositorProxy.initializeDepositPublic( + await depositor.initializeDepositPublic( fixture.fundingTx, fixture.reveal, fixture.extraData @@ -174,7 +173,7 @@ describe("TBTCDepositorProxy", () => { await bridge.sweepDeposit(fixture.expectedDepositKey) - await depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + await depositor.finalizeDepositPublic(fixture.expectedDepositKey) }) after(async () => { @@ -183,7 +182,7 @@ describe("TBTCDepositorProxy", () => { it("should revert", async () => { await expect( - depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + depositor.finalizeDepositPublic(fixture.expectedDepositKey) ).to.be.revertedWith("Deposit not initialized") }) }) @@ -192,7 +191,7 @@ describe("TBTCDepositorProxy", () => { before(async () => { await createSnapshot() - await depositorProxy.initializeDepositPublic( + await depositor.initializeDepositPublic( fixture.fundingTx, fixture.reveal, fixture.extraData @@ -206,7 +205,7 @@ describe("TBTCDepositorProxy", () => { context("when deposit is not finalized by the Bridge", () => { it("should revert", async () => { await expect( - depositorProxy.finalizeDepositPublic(fixture.expectedDepositKey) + depositor.finalizeDepositPublic(fixture.expectedDepositKey) ).to.be.revertedWith("Deposit not finalized by the bridge") }) }) @@ -230,7 +229,7 @@ describe("TBTCDepositorProxy", () => { await bridge.sweepDeposit(fixture.expectedDepositKey) - tx = await depositorProxy.finalizeDepositPublic( + tx = await depositor.finalizeDepositPublic( fixture.expectedDepositKey ) }) @@ -241,14 +240,13 @@ describe("TBTCDepositorProxy", () => { it("should remove the deposit from pending", async () => { // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect( - await depositorProxy.pendingDeposits(fixture.expectedDepositKey) - ).to.be.false + expect(await depositor.pendingDeposits(fixture.expectedDepositKey)) + .to.be.false }) it("should emit the DepositFinalized event", async () => { await expect(tx) - .to.emit(depositorProxy, "DepositFinalized") + .to.emit(depositor, "DepositFinalized") .withArgs( fixture.expectedDepositKey, expectedTbtcAmount, @@ -258,7 +256,7 @@ describe("TBTCDepositorProxy", () => { it("should return proper values", async () => { await expect(tx) - .to.emit(depositorProxy, "FinalizeDepositReturned") + .to.emit(depositor, "FinalizeDepositReturned") .withArgs(expectedTbtcAmount, fixture.extraData) }) }) @@ -277,7 +275,7 @@ describe("TBTCDepositorProxy", () => { fixture.expectedDepositKey ) - tx = await depositorProxy.finalizeDepositPublic( + tx = await depositor.finalizeDepositPublic( fixture.expectedDepositKey ) }) @@ -288,14 +286,13 @@ describe("TBTCDepositorProxy", () => { it("should remove the deposit from pending", async () => { // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect( - await depositorProxy.pendingDeposits(fixture.expectedDepositKey) - ).to.be.false + expect(await depositor.pendingDeposits(fixture.expectedDepositKey)) + .to.be.false }) it("should emit the DepositFinalized event", async () => { await expect(tx) - .to.emit(depositorProxy, "DepositFinalized") + .to.emit(depositor, "DepositFinalized") .withArgs( fixture.expectedDepositKey, expectedTbtcAmount, @@ -305,7 +302,7 @@ describe("TBTCDepositorProxy", () => { it("should return proper values", async () => { await expect(tx) - .to.emit(depositorProxy, "FinalizeDepositReturned") + .to.emit(depositor, "FinalizeDepositReturned") .withArgs(expectedTbtcAmount, fixture.extraData) }) }) @@ -330,10 +327,7 @@ describe("TBTCDepositorProxy", () => { const expectedTbtcAmount = to1ePrecision(881, 16) expect( - await depositorProxy.calculateTbtcAmountPublic( - depositAmount, - treasuryFee - ) + await depositor.calculateTbtcAmountPublic(depositAmount, treasuryFee) ).to.equal(expectedTbtcAmount) }) }) @@ -367,10 +361,7 @@ describe("TBTCDepositorProxy", () => { const expectedTbtcAmount = to1ePrecision(10, 18) expect( - await depositorProxy.calculateTbtcAmountPublic( - depositAmount, - treasuryFee - ) + await depositor.calculateTbtcAmountPublic(depositAmount, treasuryFee) ).to.equal(expectedTbtcAmount) }) }) @@ -392,7 +383,7 @@ describe("TBTCDepositorProxy", () => { const expectedTbtcAmount = to1ePrecision(98, 17) expect( - await depositorProxy.calculateTbtcAmountPublic( + await depositor.calculateTbtcAmountPublic( depositAmount, treasuryFee ) @@ -427,7 +418,7 @@ describe("TBTCDepositorProxy", () => { const expectedTbtcAmount = to1ePrecision(89, 17) expect( - await depositorProxy.calculateTbtcAmountPublic( + await depositor.calculateTbtcAmountPublic( depositAmount, treasuryFee ) @@ -462,7 +453,7 @@ describe("TBTCDepositorProxy", () => { const expectedTbtcAmount = to1ePrecision(891, 16) expect( - await depositorProxy.calculateTbtcAmountPublic( + await depositor.calculateTbtcAmountPublic( depositAmount, treasuryFee )