New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The AbstractTBTCDepositor
contract
#778
Conversation
7116e02
to
7e347dc
Compare
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.
This reference is not needed at this level as it will be used directly by child contracts.
Returning deposit key can be useful for child contracts using this function.
291bead
to
8d183bf
Compare
Here we make the abstract contract upgrade-friendly by replacing the constructor with an initializer and adding a storage gap.
DepositorProxy
contractTBTCDepositorProxy
contract
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7699766549 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7699978342 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7700019807 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7700084993 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7700167604 check. |
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"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we're not likely to switch to the latest @openzeppelin/contracts-upgradeable
v5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do it just for this abstract contract. ERC7201 seems to be worth it. Let me see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this won't be so simple. Migration from OZ v4 to v5 is non-trivial and will incur the upgrade of the OZ Upgrade plugin. This may break things while dealing with existing tBTC contract deployments. I don't want to take this risk right now.
However, to make TBTCDepositorProxy
usable with both OZ v4 and v5, I decided to remove the dependency on Initializable
in favor of a custom guard in the __TBTCDepositorProxy_initialize
initializer. Regarding the storage layout protection, I think we should keep using storage gaps in this case. That gives the best flexibility as child contracts of TBTCDepositorProxy
can still inherit from OZ v5. This is safe because ERC-7201 namespaces used by OZ v5 are safe against collisions with the standard storage layout used by Solidity (source: ERC-7201 abstract). I have also explored the possibility of using ERC-7201 in TBTCDepositorProxy
. Unfortunately, this is not an option because the OZ Upgrade Plugin raises an error for Solidity versions <0.8.20
(source) and TBTCDepositorProxy
must support all 0.8.x
versions.
Please see the change: 2c3c3c6
20c60b3
to
5744adb
Compare
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7708447506 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7714264547 check. |
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.
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7728485080 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7729702463 check. |
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 API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7730003592 check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a high-level review. The contract looks good to me with some potential minor tweaks already noted. I was not reviewing tests.
/// // embedded in the extraData. | ||
/// } | ||
/// } | ||
abstract contract TBTCDepositorProxy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not call it just TBTCDepositor
or AbstractTBTCDepositor
? The proxy suffix could suggest it is some sort of an upgradeable proxy contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for AbstractTBTCDepositor
, as the implementation could be named TBTCDepositor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to avoid pure "depositor" to not mix it with the concept of "depositor" already present in the Bridge
. I don't have strong preferences here though. AbstractTBTCDepositor
is completely fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 68d02e8
internal | ||
returns (uint256 tbtcAmount, bytes32 extraData) | ||
{ | ||
require(pendingDeposits[depositKey], "Deposit not initialized"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need pendingDeposits
mapping? I think it should be enough to check whether the deposit.depositor
value is non-zero or set to address(this)
. We retrieve IBridgeTypes.DepositRequest
anyway. Also, the implementation of the TBTCDepositorProxy
is already tightly coupled with the implementation of Bridge
and TBTCVault
. We could save some gas especially since those functions will be called by a relayer bot and not by the users.
EDIT: No, the current version is correct. We do not want _finalizeDeposit
to be called twice for the same deposit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: No, the current version is correct. We do not want _finalizeDeposit to be called twice for the same deposit.
Exactly. Moreover, we don't want to push this responsibility to the child contract.
/// 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See a6f4a92
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7744645652 check. |
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7744671760 check. |
TBTCDepositorProxy
contractAbstractTBTCDepositor
contract
Solidity API documentation preview available in the artifacts of the https://github.com/keep-network/tbtc-v2/actions/runs/7745124584 check. |
In this PR I propose some modifications to the initial implementation of `AbstractTBTCDepositor` contract (#778). ### Return initial deposit amount from _initializeDeposit function Integrators may be interested in adding more fees to the deposit amount. For this reason, it may be cleanest to calculate it based on the initial deposit amount. By returning the value here they will avoid the need to read the funding deposit amount from the storage again. ### Mock contracts tweaks I added modifications to the MockBridge and MockTBTCVault contracts, that were useful in tests of the integrator's implementation (thesis/acre#91).
Refs: #750 Here we present the smart contracts necessary to enable the L2 direct bridging (a.k.a native bridging) feature. This mechanism allows getting canonical TBTC on the given supported L2 chain, without the need to touch the L1 Ethereum chain the tBTC protocol is deployed on. Changes made as part of this pull request introduce a generic mechanism that can be deployed on all supported L2 EVM-based chains and deploy the mechanism on Ethereum Sepolia and Base Sepolia chains for testing purposes. ### Motivation Right now, a user of the supported L2 chain willing to obtain canonical L2 TBTC has to go the following path: 1. Generate a Bitcoin deposit address (using tBTC dApp or SDK) 2. Make a deposit transaction on Bitcoin 3. Reveal the Bitcoin transaction to the tBTC Bridge to start the minting process (Ethereum transaction) 4. Once TBTC is minted on Ethereum, go to the [Wormhole Token Portal](https://portalbridge.com) and bridge minted TBTC to the given L2 (another Ethereum transaction) 5. Canonical L2 TBTC lands on the user account automatically This flow is unwieldy and has major drawbacks: - It's complicated and requires multiple transactions - It requires L2 users to interact with the L1 Ethereum chain - It requires interacting with the Wormhole Token Portal The idea behind direct bridging is simplifying the above flow to something like: 1. Generate a Bitcoin deposit address (using dApp or SDK) 2. Make a deposit transaction on Bitcoin 3. Reveal the Bitcoin transaction to the tBTC Bridge **using a single transaction on the L2 chain** 4. Canonical L2 TBTC lands on the user account automatically Although this flow still relies on Wormhole underneath, the advantages are: - Simpler flow with fewer transactions - L2 users interact only with the given L2 chain - No need to touch the Wormhole Token Portal - As a next iteration, we can even get rid of the reveal deposit transaction on L2 and use Bitcoin deposit transactions as a trigger. This will improve UX even more. See the **Next iteration: Gasless bridging** section for details. ### High-level architecture The high-level architecture of the direct briding mechanism is presented on the chart below: <img width="2144" alt="bob-i1" src="https://github.com/keep-network/tbtc-v2/assets/11180469/6a32050d-6bc4-44cb-a299-1bc3e8009364"> - The **green** contracts are existing tBTC contracts that form the current bridging flow based on the Wormhole Token Portal (see [RFC 8](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-8.adoc#37-smart-contracts)). The `TBTC Bridge` component is the `Bridge` contract deployed on L1 Ethereum responsible for minting the L1 TBTC token. The `L2WormholeGateway` contract has the authority to mint canonical L2 TBTC on the given L2, based on received Wormhole L2 TBTC tokens. The `L2TBTC` component is the canonical L2 TBTC token contract. - The **blue** contracts are the new contracts that enable the new direct bridging flow. The `AbstractTBTCDepositor` contract (introduced by #778) provides some useful tooling facilitating integration with the tBTC `Bridge` and its new **deposit with extra data** function (developed in #760) which is the foundation of the L2 direct bridging mechanism. The `L1BitcoinDepositor` and `L2BitcoinDepositor` components are smart contracts handling the actual direct bridging actions on the L1 and L2 chains respectively. Those two contracts are introduced by this pull request. - The **red** contracts belong to the Wormhole protocol that handles cross-chain operations. In the context of the direct bridging mechanism, that means the transfer of minted L1 TBTC to the L2 chain. The `TokenBridge` contract handles the bookkeeping part of the transfer. The `Relayer` contract handles the actual execution of it. - The **yellow** off-chain relayer bot is the planned component (implemented as an immediate next step) that will "turn the crank" of the direct bridging mechanism. It will initialize deposits on the L1 chain (based on L2 events in the first iteration) and finalize them once L1 TBTC is minted by the tBTC `Bridge` contract. The above components interact with each other in the following way: 1. The user makes the BTC deposit funding transaction and calls the `L2BitcoinDepositor.initializeDeposit` function to initiate the deposit process on L2 (the call is made through a dApp and tBTC SDK). 2. The off-chain relayer bot observes the `DepositInitialized` event emitted by the `L2BitcoinDepositor` contract. 3. After assessing the deposit validity, the off-chain relayer bot calls the `L1BitcoinDepositor.initializeDeposit` function on L1. 4. The `L1BitcoinDepositor` contract calls the `Bridge.revealDepositWithExtra` function under the hood (through the `AbstractTBTCDepositor` abstract contract). 5. After some time (several hours), the `Bridge` contract mints L1 TBTC to the `L1BitcoinDepositor` contract. 6. The off-chain bot observes the mint event and calls `L1BitcoinDepositor.finalizeDeposit` to start the finalization process and move L1 TBTC to the L2 chain. 7. The `L1BitcoinDepositor` calls Wormhole's `TokenBridge.transferTokensWithPayload` function to initiate the cross-chain transfer of L1 TBTC. This call pulls out the L1 TBTC from the `L1BitcoinDepositor` contract and locks it in the `TokenBridge`. 8. The `L1BitcoinDepositor` calls Wormhole's `Relay.sendVaasToEvm` to send a cross-chain message to `L2BitcoinDepositor` and notify it about a pending cross-chain transfer. 9. The Wormhole protocol calls the `L2BitcoinDepositor.receiveWormholeMessages` function to deliver the cross-chain message. 10. The `L2BitcoinDepositor` contract calls the `L2WormholeGateway.receiveTbtc` function under the hood. It passes the VAA representing the cross-chain transfer as an argument of the call. 11. The `L2WormholeGateway` uses the obtained VAA to finalize the cross-chain transfer by calling the Wormhole's `TokenBridge.completeTransferWithPayload` function. This call redeems Wormhole-wrapped L2 TBTC from the `TokenBridge`. 12. The `L2WormholeGateway` uses obtained Wormhole-wrapped L2 TBTC to call `L2TBTC.mint` and mint canonical L2 TBTC. 13. Minted canonical L2 TBTC is transferred to the L2 user. ### Immediate next steps Changes presented in this pull request introduce the on-chain components of the direct bridging mechanism. To make the mechanism complete, the following steps need to take place: - Expose the L2 direct bridging feature in the tBTC Typescript SDK. This feature will be incrementally exposed for specific L2 chains the mechanism will be deployed for. The first L2 chain will be Base. - Implement the off-chain relayer bot ### Next iteration: Gasless bridging The plans for the future include some improvements to the first iteration of the direct bridging mechanism described above. Namely, the next iteration will bring gasless direct bridging that will not require any L2 transaction to initiate the process and will rely on a single Bitcoin funding transaction issued by the L2 user. On the technical level, the first two steps of the flow will be replaced by a direct call to the off-chain relayer's REST endpoint: <img width="2144" alt="bob-i2" src="https://github.com/keep-network/tbtc-v2/assets/11180469/f6a01138-0354-4b41-b3f1-4f4a38be7f91">
Refs: #750
Here we introduce the
AbstractTBTCDepositor
contract meant to facilitate integration of protocols aiming to use tBTC as an underlying Bitcoin bridge.Usage of the
AbstractTBTCDepositor
contractAn integrator willing to leverage the
AbstractTBTCDepositor
contract is supposed to:__AbstractTBTCDepositor_initialize
initializer function_initializeDeposit
and_finalizeDeposit
as part of their business logic in order to initialize and finalize deposits.Example code:
The
tbtcAmount
caveatThe
tbtcAmount
computed by_calculateTbtcAmount
and returned by_finalizeDeposit
may not correspond to the actual amount of TBTC minted for the deposit. 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 contract 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 atbtcAmount
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 fees change between the deposit minting and finalization, thetbtcAmount
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 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.