Skip to content
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

Deposit WETH and Send #1320

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Deposit WETH and Send #1320

wants to merge 2 commits into from

Conversation

alistair-singh
Copy link
Contributor

@alistair-singh alistair-singh commented Oct 22, 2024

@vgeddes
Copy link
Collaborator

vgeddes commented Oct 29, 2024

Thanks Alistair. This prototype is a good option for V1 that doesn't require any complicated Gateway contract upgrades 👍

For V2 though, probably makes sense to add a more specialized Gateway.sendEther(xcm) API that only sends ETH and an xcm to handle that Ether on AH. What do you think?

@alistair-singh
Copy link
Contributor Author

Thanks Alistair. This prototype is a good option for V1 that doesn't require any complicated Gateway contract upgrades 👍

For V2 though, probably makes sense to add a more specialized Gateway.sendEther(xcm) API that only sends ETH and an xcm to handle that Ether on AH. What do you think?

Yeah I do not think its possible to auto unwrap WETH to ETH with V1 either. I think we have to specifically build it into V2 like you said.

function transferFrom(address src, address payable dst, uint256 wad) public returns (bool) {
if (WETH9(WETH_INSTANCE).transferFrom(src, address(this), wad)) {
WETH9(WETH_INSTANCE).withdraw(wad);
dst.transfer(wad);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better change to

(bool sent, ) = dst.call{value: wad}("");
require(sent, "Failed to send funds");

}

// Unwrap WETH to ETH and transfer to destination
function transferFrom(address src, address payable dst, uint256 wad) public returns (bool) {
Copy link
Contributor

@yrong yrong Oct 31, 2024

Choose a reason for hiding this comment

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

Is this function internal or required to be public? would also suggest to remove the src as input param and replace it with msg.sender.

Copy link
Collaborator

@vgeddes vgeddes left a comment

Choose a reason for hiding this comment

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

I think the issue with introducing a wrapper token is that it fragments ETH liquidity within Polkadot, as AutoWETH and WETH have different XCM locations. It also adds confusion for developers and users about which one to use.

Auto-wrapping for Ethereum->Polkadot: We can easily implement without breaking V1 API. See the example interface further below.

Auto-unwrapping for Polkadot->Ethereum: Hard to implement this without breaking V1 compat or having to make changes on AH and BH and wait for releases.

    /// @dev Quote a fee in Ether for sending Ether
    /// 1. Delivery costs to BridgeHub
    /// 2. XCM execution costs on destinationChain
    function quoteSendEtherFee(ParaID destinationChain, uint128 destinationFee) external view returns (uint256);

    /// @dev Send Ether to parachain `destinationChain` and deposit into account `destinationAddress`
    ///
    /// Params:
    /// * `amount`: The amount of ether to send
    /// 
    /// The Ether passed in the transaction as `msg.value` must cover both `amount` as well the bridging
    /// fee calculated by `quoteSendEtherFee`.
    function sendEther(
        ParaID destinationChain,
        MultiAddress calldata destinationAddress,
        uint128 destinationFee,
        uint128 amount
    ) external payable;

We should definitely support full auto-(un)wrap for v2 though.

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