Skip to content

Implement tBTC Bridge fees reimbursement #942

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

Merged
merged 19 commits into from
Jun 17, 2025
Merged

Conversation

nkuba
Copy link
Collaborator

@nkuba nkuba commented Apr 23, 2025

For deposits below a certain threshold, the fees will be reimbursed from the fees reimbursement pool, funded by the Acre protocol.

The tBTC Bridge fees reimbursement threshold can be updated by the Acre protocol owner.

For deposits below a certain threshold, the fees will be reimbursed
from the fees reimbursement pool, funded by the Acre protocol.

The tBTC Bridge fees reimbursement threshold can be updated by the
Acre protocol owner.
@nkuba nkuba requested a review from pdyraga April 23, 2025 09:10
Copy link

netlify bot commented Apr 23, 2025

Deploy Preview for acre-dapp-testnet canceled.

Name Link
🔨 Latest commit 72b5571
🔍 Latest deploy log https://app.netlify.com/sites/acre-dapp-testnet/deploys/6808ae7fae97750008ed478f

Copy link

netlify bot commented Apr 23, 2025

Deploy Preview for acre-dapp canceled.

Name Link
🔨 Latest commit 72b5571
🔍 Latest deploy log https://app.netlify.com/sites/acre-dapp/deploys/6808ae7feb0bcd0008683d5a

Copy link
Collaborator

@pdyraga pdyraga left a comment

Choose a reason for hiding this comment

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

Looks good so far!

Comment on lines 49 to 57
uint256 availableBalance = IERC20(tbtcToken).balanceOf(address(this));

if (availableBalance < reimbursedAmount) {
reimbursedAmount = availableBalance;
}

if (reimbursedAmount > 0) {
IERC20(tbtcToken).safeTransfer(msg.sender, reimbursedAmount);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Because there is some logic to calculate the reimbursed amount (I mean, in some cases the full amount cannot be covered), maybe we can create a view function so we can call it in SDK and use it in this function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the dApp it should be enough to check the FeesReimbursementPool contract balance. We will need to think about a value below which we won't show the info about fee reimbursements in the UI. Since the bridging operation is asynchronous, there will be a delay between the moment the user initiates a deposit in the UI and when the bridging is finalized and the fee is reimbursed.

It is not required to initialize the value as 0 is the default value anyway.
Copy link

netlify bot commented Jun 12, 2025

Deploy Preview for acre-dapp ready!

Name Link
🔨 Latest commit 819fe69
🔍 Latest deploy log https://app.netlify.com/projects/acre-dapp/deploys/6851dc890622240008525c3a
😎 Deploy Preview https://deploy-preview-942--acre-dapp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

netlify bot commented Jun 12, 2025

Deploy Preview for acre-dapp-testnet ready!

Name Link
🔨 Latest commit 819fe69
🔍 Latest deploy log https://app.netlify.com/projects/acre-dapp-testnet/deploys/6851dc895ec5bd0008d881b2
😎 Deploy Preview https://deploy-preview-942--acre-dapp-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

This contract has to be updated with the recent changes in the BitcoinDepositor contract.
@nkuba nkuba force-pushed the deposit-fees-reimbursement branch from 5b2f253 to 831ef55 Compare June 12, 2025 21:47
nkuba and others added 13 commits June 13, 2025 00:05
This function allows to update the fees reimbursement pool address.
This commit adds unit tests for new setters added to the BitcoinDepositor contract.
The deposit amount for reimbursement is inclusive of the threshold amount.
Check if fees reimbursement pool is set before calling reimburse
function to avoid meaningless revert.
Tests assume this reference is returned but it was not. This change
should fix the tests.
Return Withdrawn and Reimbursed events from appropriate functions. In
theory, we could track it with tBTC Transfer events but in practice,
more specific events make state change tracking much easier.
A complete set of unit tests for FeesReimbursementPool contract.
Check if the provided tBTC and BitcoinDepositor contract addresses are
non-zero when the FeesReimbursementPool contract is initialized.
Dropped the "copy" suffix from the file name. Something committed
accidentally in the past.
The FeesReimbursementPool address in the BitcoinDepositor can be set by
the script only on Hardhat network. On all other networks it has to be
done manually by the governance.

Accepting ownership of FeesReimbursementPool can only happen on Hardhat
network. On all other networks, the governance address need to call this
function and the governance is either external EOA (Sepolia) or a
multisig (Ethereum mainnet).
The function returns a value that has been actually reimbursed, which is
important to use in integration with the FeesReimbursementPool contract.
@pdyraga pdyraga marked this pull request as ready for review June 17, 2025 10:01
nkuba added 2 commits June 17, 2025 12:55
This is a pattern we use across the file.
@pdyraga pdyraga merged commit d902c07 into main Jun 17, 2025
31 checks passed
@pdyraga pdyraga deleted the deposit-fees-reimbursement branch June 17, 2025 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants