-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Fee to OriginalTokenBridge #6
base: main
Are you sure you want to change the base?
Conversation
const withdrawalFee = (amount * BigInt(withdrawalFeeBps)) / BigInt(10000) / LDtoSD | ||
|
||
await originalTokenBridge.connect(owner).withdrawFee(originalToken.target, owner.address, withdrawalFee) | ||
expect(await originalToken.balanceOf(owner.address)).to.be.eq(withdrawalFee) |
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.
Can you check the following:
when fee is 1%, while bridging X user receives 99%, the fees are 1%.
I see you only checked the fee part
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.
@leonprou I have updated the test which checks the total value locked for a token which is essentially the actual amount of token sent in the LZ message to be minted on the dest chain
expect(await originalToken.balanceOf(owner.address)).to.be.eq(withdrawalFee) | ||
}) | ||
|
||
it("bridges WETH and withdraws fees", async () => { |
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.
that's actually bridging native, no?
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.
yes, this is what we will be using for FUSE token
@@ -107,13 +119,20 @@ contract OriginalTokenBridgeUpgradable is TokenBridgeBaseUpgradable { | |||
require(to != address(0), "OriginalTokenBridge: invalid to"); | |||
_checkAdapterParams(remoteChainId, PT_MINT, adapterParams); | |||
|
|||
uint withdrawalAmountLD = amountLD; | |||
if (withdrawalFeeBps > 0) { | |||
uint withdrawalFee = (amountLD * withdrawalFeeBps) / TOTAL_BPS; |
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.
Please check this flow with max and min values for uint
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.
@leonprou why is that needed?
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 user sends a really big amount of tokens, when it gets multiplied by withdrawalFeeBps
an integer overflow might happen.
No description provided.