Skip to content
This repository has been archived by the owner on Apr 12, 2021. It is now read-only.

Extend finalizeWithdrawal in Standard Bridge Interface #355

Open
krzkaczor opened this issue Apr 1, 2021 · 6 comments
Open

Extend finalizeWithdrawal in Standard Bridge Interface #355

krzkaczor opened this issue Apr 1, 2021 · 6 comments

Comments

@krzkaczor
Copy link
Contributor

Problem Description
As a developer of a 3rd party (external to a birdge) fast withdrawal solution I would like to have a way to connect an original withdrawal requester from an l2 to l1 message payload. Additionally, I would like to embed even more details about the withdrawal request (fees etc).

Solution

Add extraData field to iOVM_L1TokenGateway's finalizeWithdrawal. This adds more context to cross-chain messages and allows to build 3rd party fast withdrawal solutions OUTSIDE of the token bridge.

extraData can contain a hash of a message containing the address of an original requester, info about fees etc.

Additional context
FW solution flow possible after this change:

  1. Alice calls withdrawTo on L2DepositedToken with an address of a smartcontract coordinating FW let's call it Coordinator.
  2. A liqudity provider Bob validates that withdrawal is correct.
  3. Bob sends Alice funds on L1 by letting Coordinator to send his funds to Alice and record this fact. Alice can enjoy their token on L1.
  4. When Alice's withdrawal settles, Bob claims it as he can proof that he already sent Alice funds.

If (3) never happened, after withdrawal settles, Alice proofs that she initiated the original withdraw and she claims tokens. Without the change described in this PR a cross-chain message only contains address of a Coordinator SC (to field) and hence Alice can not prove that she originated the withdrawal.

PoC of such solution was implemented by @gakonst in: https://github.com/gakonst/optimistic-fast-withdrawals/blob/feat/fast-withdrawals/contracts/MarketMaker.sol

Alice and Bob can negotiate additional fees in a state channel and they don't have to be part of the cross-chain message.

Original discussion: #349

@gakonst
Copy link
Contributor

gakonst commented Apr 2, 2021

One thing to note: The PoC is incorrect/unsafe because the message being validated allows anybody to specify to. The correct message would be one which commits to the withdrawal's receiver being the MM address (MUST be immutable) AND any additional data the user provided (e.g. the actual receiver, any fees they want etc.).

bytes memory data = abi.encode(to, (address))
bytes memory call = abi.encodeWithSelector(L2ERC20.withdrawTo.selector, address(this), amount, data);

and client side the user would submit the withdrawal as done here, but by also encoding their data (js pseudocode below):

// user specifies their address in the extraData because their address is not passed to the L1 messenger
// due to the call-flow being: User -> L2ERC20 -> L2Messenger -> L1Messenger, meaning that the l2_msgsender
// is L2ERC20
withdrawalTx = await L2_ERC20.connect(l2account1).withdrawTo(
        marketMaker.address,
        amount,
        abi.encode(['address'], [await l2account1.getAddress()]
)

Separately from that, the PoC is missing features like greenlightMany, and allowing the MM to specify fees, but that should be downstream of getting the above implemented.

I think that this approach is the simplest one for getting Fast Withdrawals minimally enabled, without introducing any additional mechanisms/overheads. The client side watcher for this should also be pretty simple:

  • Define an L1 and L2 endpoint,
  • watch for WithdrawalInitiated events on L2 for your token(s) (or filter for SentMessage on the L2 relay which come from the L2 tokens you're watching)
  • check if to is your MM contract address and decode the data
  • call greenlight with the parameters specified, and set a T timer
  • once T passes, call claim on the L1 contract

What's nice about this is that it requires 1 L2 transaction for the user w/o any additional approvals, and 1 L2 transaction & 1 L1 transaction by the market maker.

@transmissions11
Copy link
Contributor

transmissions11 commented Apr 2, 2021

@gakonst I believe it's possible to implement your PoC bridge without this addition if you used 2 xDomain messages right? One where you send the tokens (with no context) and a second one with a custom call using the xDomainMessenger to call a function on your L1 Market Maker contract like registerTokenTransferContext() which could contain the additional context?

I would still much rather have an extraData field, just confirming that I'm thinking about things correctly 😅

@transmissions11
Copy link
Contributor

Ideally we could have an extraData field not only on finalizeWithdrawal but on finalizeDeposit too. There are cases where a contract on L2 might get sent tokens from L1 but only want to allow them to be withdrawn by users after some condition is met.

Specifying the condition is currently not possible (unless my assumption above is correct that you could do this with 2 calls).

@gakonst
Copy link
Contributor

gakonst commented Apr 2, 2021

Re 1: Uh sure it's prob possible, but does not sound like great UX
Re 2: Agreed

@transmissions11
Copy link
Contributor

We seem to agree here. Anyone want to pick up @gakonst's PR again?

@ben-chain
Copy link
Collaborator

I have a plan in motion to collect this and other feature requests for the bridge so that we can get a proper community-driven implementation, stay tuned :)

I have been thinking about another bit of devex here which you folks might have a good grasp on. I was wondering--would it be useful to expose the unique identifier for cross-chain messages more readily in the messengers and token gateways? For example, sendMessage could return a messageHash or nonce, and then the gateways could return that back through initializeDeposit/Withdrawal. Seems like this might be useful info to be able to access, but I'm not positive that it actually is. Any thoughts?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants