Skip to content

Conversation

cyl19970726
Copy link

Signed-off-by: cyl19970726 [email protected]

Copy link
Contributor

@qizhou qizhou left a comment

Choose a reason for hiding this comment

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

Should BridgeDestination.withdraw() also handle the native token logic?

}

function buy(TransferKey memory tkey) public {
function buy(TransferKey memory tkey) public payable{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: payable{ => payable {

@qizhou
Copy link
Contributor

qizhou commented Dec 23, 2021

Further, please use

npm run prettier:fix

to auto-format the code.

) {
return transferData.fee;
} else {
return transferData.fee * (currentTime - transferData.startTime);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return transferData.fee * (currentTime - transferData.startTime);
return transferData.fee * (currentTime - transferData.startTime) / transferData.feeRampup;

Copy link
Contributor

Choose a reason for hiding this comment

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

Not yet fixed?


if (tkey.transferData.tokenAddress == address(0)) {
require(msg.value >= amount, "not enough msg.value");
payable(tkey.transferData.destination).transfer(amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

may need nonReentrant to avoid reentrancy attack?

cyl19970726 and others added 2 commits December 24, 2021 13:32
modify BridgeDestination.getLPFee()

Co-authored-by: Qi Zhou <[email protected]>
add nonReentrant to BridgeDestination.withdraw() and BridgeDestination.buy()

Signed-off-by: cyl19970726 <[email protected]>
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.

2 participants