ETH-81: Expose transportSignatures for an alternative withdraw flow #239

jtakalai · 2021-03-30T09:23:23Z

New withdraw functionality.

Alpha version is already in NPM, this PR is just adding tests. After that it should be good for master.

linear · 2021-03-30T09:23:25Z

ETH-81 Expose DU2 functions for signature transport

Swash withdraw pattern:

browser plugin sends HTTP request to server saying "X wants to withdraw to address A in mainnet, here's the signature"
server calls DataUnion.withdraw* function and returns the message-hash M in HTTP reply
browser plugin polls the AMB, if the signatures have been collected for message-hash M
after signatures have been collected, browser plugin will call DataUnion.transportSignaturesForMessage

Expose the necessary functions for each step of that flow:

(signatures already exist)
in freeWithdraw mode, return the messageHash(es) from the withdraw promise
- split transportSignaturesForTransaction into getMessageHashes and move rest of code to _executeWithdraw (for !freeWithdraw), figure out what to do with messageId and whether transportSignaturesForMessage should check if signatures already were transported (required messageId, MAYBE)
expose requiredSignaturesHaveBeenCollected
expose transportSignaturesForMessage

because the method to do it is transportMessage and that's really all it means, so either it should be the SAME name, or else a bit more descriptive

also using describe.each instead of the homebrew loop. WebStorm recognizes this syntax, so please never use homebrew loops!

apparently ethers doesn't do enough to tell tx.wait() returns a ContractReceipt...

strange that `instanceof String` doesn't actually catch a string

for some reason it was polling every 100ms, which is definitely too much in production

tests should pass now (fingers crossed)

all tests timed out for some reason; could be that now that there are so many cases (5 x 4, I think) it requires the whole thing to run under 5min, which it might not TODO: turn it down to what's actually needed

bridge timeout probably too low for real world?

also TrReceipt -> ContractReceipt was changed in this PR

making choices based on fairly arbitrary line length limits feels bad man. Adding eslint-disables also feels bad. But I acknowledge it's good to limit line length, in general. I guess.

Date.now() feels a bit hacky, running count is better

All tests run in parallel. Even trying to start bridge "for those cases that need it" ends up bridge paying for other transports as well and every test case being confused. Also for some reason the transport doesn't even happen to the cases that want it. Augh.

jtakalai · 2021-04-28T06:00:50Z

Data union tests are all green. Merging after a review.

just to guard against the super improbable failure of shared ids in parallel-run tests

src/dataunion/DataUnion.ts

timoxley

Would like it if more functions took options objects instead of positional parameters, my IDE doesn't have the nice positional naming stuff that yours does, but otherwise LGTM.

test/integration/dataunion/withdraw.test.ts

src/dataunion/DataUnion.ts

test/integration/dataunion/withdraw.test.ts

src/dataunion/DataUnion.ts

format strings are MUCH nicer for a lot of things: they show the JS type, also pretty-print objects (no need to JSON.stringify)

One is the property of the bridge, one is the property of the function call, it's like the opposite angle of looking at it. But I'll still change it so that the name is the same. So freeWithdraw -> !payForTransport everywhere. It's better that way.

ETH-81: Expose transportSignatures for an alternative withdraw flow

b10fe81

Juuso Takalainen added 13 commits March 30, 2021 12:36

make eslint happy

99f2abf

Update README with two-(three?)step withdraw

5e0a208

fix tables

b52a2ec

Merge branch 'master' into ETH-81-expose-signature-transport

5091cf3

transportSignature -> payForTransport

42d99e8

because the method to do it is transportMessage and that's really all it means, so either it should be the SAME name, or else a bit more descriptive

Added tests for each of the withdraw patterns documented in README

de6bbec

also using describe.each instead of the homebrew loop. WebStorm recognizes this syntax, so please never use homebrew loops!

added type

501a15b

apparently ethers doesn't do enough to tell tx.wait() returns a ContractReceipt...

fix timing issue

33b1715

added todo ticket number, cleanup

ad297a4

fix broken type test

02d4dbb

strange that `instanceof String` doesn't actually catch a string

Better error message

01840c3

tweak timings

8ab7940

for some reason it was polling every 100ms, which is definitely too much in production

fix the bridge-sponsored withdraw case

178a8e1

tests should pass now (fingers crossed)

jtakalai requested review from teogeb and timoxley April 22, 2021 07:53

Juuso Takalainen added 13 commits April 22, 2021 13:16

cleanup

1e6b1a6

longer timeout

cdfedc2

all tests timed out for some reason; could be that now that there are so many cases (5 x 4, I think) it requires the whole thing to run under 5min, which it might not TODO: turn it down to what's actually needed

timeout tweaks

13b20b3

Improve logging

670377c

bridge timeout probably too low for real world?

these types are in public APIs

acc7157

also TrReceipt -> ContractReceipt was changed in this PR

typo fix

388fd70

make lint happy

a502204

making choices based on fairly arbitrary line length limits feels bad man. Adding eslint-disables also feels bad. But I acknowledge it's good to limit line length, in general. I guess.

Merge branch 'master' into ETH-81-expose-signature-transport

f4567bb

clean up old merged PR 241

8db8d5b

small fix

aeded55

deterministic test wallet generation

fbed773

Date.now() feels a bit hacky, running count is better

Fix bridge, refactor out getBalance argument

8ebafc3

Juuso Takalainen added 3 commits April 27, 2021 12:32

Fix bridge whitelisting inside the test

f21cd2b

Merge branch 'master' into ETH-81-expose-signature-transport

00a5884

fix ETH-81: engine-and-editor -> core-api

aebff64

Remove the rest of Date.now() pseudo-ids

b0a6152

just to guard against the super improbable failure of shared ids in parallel-run tests

teogeb reviewed Apr 28, 2021

View reviewed changes

src/dataunion/DataUnion.ts Show resolved Hide resolved

teogeb reviewed Apr 28, 2021

View reviewed changes

src/dataunion/DataUnion.ts Show resolved Hide resolved

teogeb reviewed Apr 28, 2021

View reviewed changes

src/dataunion/DataUnion.ts Outdated Show resolved Hide resolved

timoxley approved these changes Apr 28, 2021

View reviewed changes

Juuso Takalainen added 3 commits April 30, 2021 09:45

logging overhaul

9f32fda

format strings are MUCH nicer for a lot of things: they show the JS type, also pretty-print objects (no need to JSON.stringify)

freeWithdraw -> !payForTransport

893409a

One is the property of the bridge, one is the property of the function call, it's like the opposite angle of looking at it. But I'll still change it so that the name is the same. So freeWithdraw -> !payForTransport everywhere. It's better that way.

Parallelized some blockchain operations that take an HTTP call

c0105ca

jtakalai merged commit 108afd3 into master Apr 30, 2021

jtakalai deleted the ETH-81-expose-signature-transport branch April 30, 2021 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ETH-81: Expose transportSignatures for an alternative withdraw flow #239

ETH-81: Expose transportSignatures for an alternative withdraw flow #239

Uh oh!

jtakalai commented Mar 30, 2021 •

edited

Loading

Uh oh!

linear bot commented Mar 30, 2021

Uh oh!

jtakalai commented Apr 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timoxley left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ETH-81: Expose transportSignatures for an alternative withdraw flow #239

ETH-81: Expose transportSignatures for an alternative withdraw flow #239

Uh oh!

Conversation

jtakalai commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear bot commented Mar 30, 2021

Uh oh!

jtakalai commented Apr 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timoxley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jtakalai commented Mar 30, 2021 •

edited

Loading