Skip to content

Conversation

karim-en
Copy link
Collaborator

@karim-en karim-en commented May 8, 2024

The refund for the ft_transfer_call that has been introduced in this PR led to additional vulnerabilities, so we decided to remove this feature of auto refund.

@karim-en karim-en requested review from kiseln, kisialiou and olga24912 May 8, 2024 15:36
Copy link

@kiseln kiseln left a comment

Choose a reason for hiding this comment

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

Makes sense

/// * A `U128` value representing the amount that was withdrawn, or `0` if the promise was not
/// successful and the funds were returned to the user's balance.
#[private]
pub fn withdraw_callback(
Copy link

Choose a reason for hiding this comment

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

I think this function also needs to be removed from FastBridgeInterface

if let Some(msg) = msg {
self.call_ft_transfer_call(token_id, amount, sender_id, recipient_id, msg)
ext_token::ext(token_id)
.with_static_gas(utils::tera_gas(50))
Copy link
Contributor

Choose a reason for hiding this comment

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

Use const for provided gas

self.call_ft_transfer_call(token_id, amount, sender_id, recipient_id, msg)
ext_token::ext(token_id)
.with_static_gas(utils::tera_gas(50))
.with_attached_deposit(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

use const

ext_token::ext(token_id)
.with_static_gas(utils::tera_gas(5))
.with_attached_deposit(1)
.ft_transfer(recipient_id, amount, memo)
Copy link
Contributor

Choose a reason for hiding this comment

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

we also should emit FastBridgeWithdrawEvent

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