One-transaction TBTC to BTC #302
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Modified `TBTCVault.receiveApproval` function to allow unminting TBTC and requesting Bank balance redemption in the Bridge in a single transaction via `TBTC.approveAndCall`. Also, added `TBTCVault.redeem` function allowing to unmint TBTC and request redemption in the `Bridge` on an already approved TBTC balance. During the work on this feature, I have been exploring different options, including exposing `IBridge` interface that would be called from `Bank` or `TBTCVault` to redeem balance. The current version won because it does not tightly-couple vault implementations with the specific `Bridge` implementation. For example, the second version of the `Bridge` may not even have a concept of a wallet or a main wallet UTXO. Passing parameters as bytes is more flexible and uses an existing `Bank` mechanism of `receiveBalanceApproval`.
This change fixes mysterious problem in unit tests. Let's say there is a unit test calling `await waffle.loadFixture(bridgeFixture)` in the setup. For example, `Bridge.Wallets.test.ts`. Then, `VendingMachine.test.ts` loads `await waffle.loadFixture(vendingMachineFixture)` and modifies the state of `VendingMachine` by `transferVendingMachineUpgradeInitiatorRole` to some address. Finally, some other test starts and that test calls `await waffle.loadFixture(bridgeFixture)`. For example, `TBTCVault.Redemption.test.ts`. THIS TEST WILL OBSERVE CHANGES DONE BY `VendingMachine.test.ts`. For example, the upgrade initiator will be the address set by `VendingMachine.test.ts`. This is not happening if we use `await waffle.loadFixture(bridgeFixture)` in all three tests. The root cause is unkown, we suspect it is some clash between plugins. There will be a separate issue opened to investigate it. For now we fix it by loading `bridgeFixture` which makes sense for all tests - this is our full system deployment.
This change allows to lower the gas consumption. Before/after: | Bank · approveBalanceAndCall · 33039 · 120387 · 78764 │ | Bank · approveBalanceAndCall · 32769 · 120113 · 78535 │ | TBTCVault · redeem · 161704 · 178643 · 168753 │ | TBTCVault · redeem · 161704 · 178643 · 168753 │
Added @dev section making clear what are the requirements in regards to approved amount of TBTC when calling `TBTCVault.redeem` function.
Improved unit tets for `redeem` and `receiveApproval` functions adding case for P2SH redeemer script.
*Unminting* is when TBTC is transformed to Bank balance. *Redemption* is when Bank balance is transformed to BTC. Having this in mind, former `redeem` function should be called `unmintAndRedeem` because it transforms TBTC to BTC.
lukasz-zimnoch
approved these changes
May 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #256
Modified
TBTCVault.receiveApprovalfunction to allow unminting TBTC andrequesting Bank balance redemption in the Bridge in a single transaction via
TBTC.approveAndCall. Also, addedTBTCVault.redeemfunction allowing tounmint TBTC and request redemption in the
Bridgeon an already approved TBTCbalance.
During the work on this feature, I have been exploring different options,
including exposing
IBridgeinterface that would be called fromBankorTBTCVaultto redeem balance. The current version won because it does nottightly-couple vault implementations with the specific
Bridgeimplementation.For example, the second version of the
Bridgemay not even have a concept ofa wallet or a main wallet UTXO. Passing parameters as bytes is more flexible and
uses an existing
Bankmechanism ofreceiveBalanceApproval.The reasoning behind the fix in 9738385 is explained in the commit message.
I have also captured the potential investigation work in #303.