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

Support openChannelWithDeposit from new contracts #2919

Merged
merged 5 commits into from
Sep 6, 2021

Conversation

andrevmatos
Copy link
Contributor

@andrevmatos andrevmatos commented Aug 31, 2021

Fixes #2891

Short description
raiden-contracts 0.39 supports a new method: openChannelWithDeposit, which does channel open + deposit in a single transaction, more efficient and faster to confirm than separate channelOpen + channelDeposit. This PR dynamically uses this method iff contracts being used support it, falling back to normal open + confirmation + deposit + confirmation as a fallback (so we can still suport 0.37 and 0.39+ contracts on the same codebase).
This is transparent, and automatically done when deposit is requested together with open, if possible.

A small edge case to keep in mind (and tested by SP, and now with tests specific for it) is when two peers try to openChannelWithDeposit with each other at the same time; only one of the txs will succeed, making the "open" side of it to succeed for both clients but "deposit" only for the side which won the race; in this case, the losing side will detect it and fallback to emitting the channelDeposit.request as it was with the old mechanism, completing the deposit and succeeding the open + deposit operation, as expected by the user.

Definition of Done

  • Steps to manually test the change have been documented
  • Acceptance criteria are met
  • Change has been manually tested by the reviewer (dApp)

Steps to manually test the change (dApp)

@andrevmatos andrevmatos added sdk 🖥 optimization ⚡ Optimizations for the implementation or protocol labels Aug 31, 2021
@andrevmatos andrevmatos self-assigned this Aug 31, 2021
@codecov
Copy link

codecov bot commented Aug 31, 2021

Codecov Report

Merging #2919 (5883883) into master (a68e2ff) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2919      +/-   ##
==========================================
+ Coverage   93.57%   93.66%   +0.08%     
==========================================
  Files         116      116              
  Lines        6369     6394      +25     
  Branches     1149     1150       +1     
==========================================
+ Hits         5960     5989      +29     
+ Misses        354      350       -4     
  Partials       55       55              
Flag Coverage Δ
dapp 80.57% <ø> (ø)
dapp.unit 80.57% <ø> (ø)
sdk 95.72% <100.00%> (+0.09%) ⬆️
sdk.e2e 72.97% <88.88%> (-0.39%) ⬇️
sdk.integration 79.76% <100.00%> (+0.18%) ⬆️
sdk.unit 49.18% <22.22%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
raiden-ts/src/transfers/epics/utils.ts 100.00% <ø> (ø)
raiden-ts/src/channels/epics/deposit.ts 91.30% <100.00%> (-0.37%) ⬇️
raiden-ts/src/channels/epics/open.ts 100.00% <100.00%> (+3.57%) ⬆️
raiden-ts/src/channels/utils.ts 100.00% <100.00%> (+2.46%) ⬆️
raiden-ts/src/services/epics/udc.ts 100.00% <100.00%> (ø)
raiden-ts/src/transfers/epics/coopsettle.ts 95.58% <100.00%> (+0.06%) ⬆️
raiden-ts/src/transfers/epics/withdraw.ts 94.96% <100.00%> (+0.03%) ⬆️
raiden-ts/src/utils/ethers.ts 91.42% <100.00%> (+0.70%) ⬆️
raiden-ts/src/transport/epics/messages.ts 94.67% <0.00%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a68e2ff...5883883. Read the comment docs.

The later is simpler, receives saner parameters and includes more shared
logic, like fetching balanceOf and allowance from token contract.
If channelOpen was requested with deposit and we're on the new
contracts, instead of emitting a parallel channelDeposit.request with
waitOpen=true, we ensureApprovedBalance$ inside channelOpenEpic and call
`openChannelWithDeposit` directly, which should open and deposit in a
single tx (faster); `channelEventsEpic` is able to handle deposits on
the same block as open.
@andrevmatos andrevmatos force-pushed the feat/openChannelWithDeposit branch 2 times, most recently from 8622892 to acac79e Compare September 4, 2021 18:58
@andrevmatos andrevmatos marked this pull request as ready for review September 4, 2021 19:35
Copy link
Contributor

@palango palango left a comment

Choose a reason for hiding this comment

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

Great!

@andrevmatos andrevmatos merged commit e1ecfe6 into master Sep 6, 2021
@andrevmatos andrevmatos deleted the feat/openChannelWithDeposit branch September 6, 2021 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization ⚡ Optimizations for the implementation or protocol sdk 🖥
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use TokenNetwork.openChannelWithDeposit
2 participants