GridPlus / cryptobridge-contracts

Smart contracts for trustless bridges
MIT License
75 stars 31 forks source link

Upgrade withdrawal functions #11

Closed alex-miller-0 closed 6 years ago

alex-miller-0 commented 6 years ago

This is a bounty associated with the Relay.sol contract. Currently, there are three functions that make up a full withdrawal:

  1. prepWithdraw makes a Merkle-Patricia proof given transaction data and saves a temporary Withdrawal struct to storage. This is the most expensive function because it stores several pieces of data.

  2. proveReceipt takes receipt data and forms a Merkle-Patricia proof. If successful, it saves the receiptsRoot for the block to storage.

  3. withdraw forms a standard Merkle proof given [modified] headers (they are modified by the cryptobridge client to contain a subset of the Ethereum header data). If successful, this triggers an ERC20 transfer and deletes the temporary Withdrawal struct associated with the sender.

This process takes about 500,000 gas and requires three transactions on the withdrawal chain. For the next version, I would like to cut that gas usage by as much as possible and reduce it down to one transaction. This is where you come in.

Requirements:

A successful proposal will do the following:

  1. Reduce the gas consumption by a significant quantity
  2. Reduce the number of withdrawal transactions from 3 to 1. If this cannot be done, it must show evidence of that. A reduction from 3 to 2 is also acceptable if there is evidence it cannot be reasonably reduced to 1.
  3. Update the test cases in bridge.js to call the updated API with new data.
  4. Update the README to reflect API changes

Hints

Any method to perform the above requirements is fine, but I would suggest looking at passing larger bytes arrays and slicing them with utility functions, perhaps in an external library. This would require a considerable rewrite of prepWithdraw, which has the arguments pre-sliced. For an example of bytes slicing, you can look at the way logs are sliced in proveReceipt.

gitcoinbot commented 6 years ago

This issue now has a funding of 0.7 ETH (656.6 USD) attached to it.

alex-miller-0 commented 6 years ago

Don't worry about the 7 day expiration on this one; it is valid until March 15 at midnight UTC. Also, 'hours' is a stretch unless you're an expert in solidity.

gitcoinbot commented 6 years ago

Work has been started on the 0.7 ETH (674.8 USD) funding by:

  1. @lazaridiscom

    Please work together and coordinate delivery of the issue scope. Gitcoin doesn't know enough about everyones skillsets / free time to say who should work on what, but we trust that the community is smart and well-intentioned enough to work together. As a general rule; if you start work first, youll be at the top of the above list ^^, and should have 'dibs' as long as you follow through.

    On the above list? Please leave a comment to let the funder (@alex-miller-0) and the other parties involved what you're working, with respect to this issue and your plans to resolve it. If you don't leave a comment, the funder may expire your submission at their discretion.

alex-miller-0 commented 6 years ago

@lazaridiscom

You mean the usual git clone

I meant just copy the contracts if you want to use them as an import, since I can't get ethpm to publish.

Why this requirement? Shouldn't truffle handle this (as an development/automation tool) ?

It's just a troubleshooting mechanism. Not a requirement.

I sense that the overall task can make a full rework (or even rewrite) of the code neccessary. Would it be in any way possibly to split this into independent sub-tasks?

Sure if you want to do a series of PRs and link them up here that's fine. In that case I had put out a bounty on a blocking feature that I ended up taking on myself. This upgrade is not blocking, so I don't foresee the same problem.

gitcoinbot commented 6 years ago

Work has been started on the 0.7 ETH (295.63 USD @ $422.33/ETH) funding by:

  1. @lazaridiscom

    Please work together and coordinate delivery of the issue scope. Gitcoin doesn't know enough about everyones skillsets / free time to say who should work on what, but we trust that the community is smart and well-intentioned enough to work together. As a general rule; if you start work first, youll be at the top of the above list ^^, and should have 'dibs' as long as you follow through.

    On the above list? Please leave a comment to let the funder (@alex-miller-0) and the other parties involved what you're working, with respect to this issue and your plans to resolve it. If you don't leave a comment, the funder may expire your submission at their discretion.

owocki commented 6 years ago

👋 hi from gitcoin here.. sorry for the bot duplication hiccups.. just deployed a fix for them on a go-forward basis.

alex-miller-0 commented 6 years ago

Pending token 0x..0 means the prepWithdraw function failed (because nothing was stored)

No rush on this! Appreciate your hard work getting set up.

owocki commented 6 years ago

@lazaridiscom you cannot do a submission bc the bounty has expired.. sorry! this is something were working on to make more clear in the flow (and maybe even make optional).

we will need to have the submitter kill the bounty and then pay out your ETH address directly if they agree that the submission is ready to accepted and they don't mind that it's expired.

alex-miller-0 commented 6 years ago

Last time I killed an expired bounty I was shown the option to issue the reward regardless of the expiration. Is that no longer an option?

owocki commented 6 years ago

@alex-miller-0 great question.. unfortunately, this is a piece of functionality that does not exist after the migration to standard bounties.. its something we are considering bringing back. is this something that's important for you?

alex-miller-0 commented 6 years ago

Yes, I'd really like to have that functionality. I actually don't see any way to end the issue at all. How are expired issues resolved now?

owocki commented 6 years ago

Yes, I'd really like to have that functionality. I actually don't see any way to end the issue at all.

(mind hard refreshing before you do this? i just deployed a change) if you are logged into the mainnet with 0xaf98e3bf3ef59207783f11e2822541023ed39e8b (the funder) as your web3.eth.coinbase and viewing https://gitcoin.co/funding/details?url=https://github.com/GridPlus/cryptobridge-contracts/issues/11 you should now see a 'kill bounty' button

How are expired issues resolved now?

I'm not sure I follow the question... Do you mean how do you clawback funds? Or how do you pay the work submitter?

alex-miller-0 commented 6 years ago

Okay, I see the kill button. I'm assuming this sends the funds back to me?

@lazaridiscom what's your ETH address?

owocki commented 6 years ago

Okay, I see the kill button. I'm assuming this sends the funds back to me?

yep!

owocki commented 6 years ago

@lazaridiscom updating the issue status to be 'done' instead of 'expired' now

@alex-miller-0 when i update the bounty status to preverse stats per @lazaridiscom request above, the 'kill bounty' link will go away, but you can always access it here if need be => https://gitcoin.co/funding/kill?url=https://github.com/GridPlus/cryptobridge-contracts/issues/11

alex-miller-0 commented 6 years ago

@lazaridiscom I'm going to kill the bounty and send you 0.7 ETH out of the Grid+ bounty wallet (0xaf98e3bf3ef59207783f11e2822541023ed39e8b)

alex-miller-0 commented 6 years ago

https://etherscan.io/tx/0x536a53543369b9c24bc0ced315867097492fbcbdc7e2fcfece2fc24e8444254e

Thanks for your work!