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

Enable faster channel opening & deposit by parallelizing them and their confirmations #1128

Merged
merged 2 commits into from
Mar 4, 2020

Conversation

andrevmatos
Copy link
Contributor

Currently, channel open & deposit can take a very long time, since they're called separatelly from userspace and needs to be processed serially, as deposit can only be done after channel state is in place.
But the information needed for deposit is available while opening the channel (when the respective txs are sent and mined), and therefore, we can parallelize these operations (and specially their confirmation) to minimize the time needed to open a channel.
So, this PR:

  • makes channelOpen.request accept an optional deposit param
  • if requesting a deposit with said param, call token.approve in parallel with openChannel
  • wait for both to be mined in parallel
  • iff channel open was successful, wait for the channelId and right away call setTotalDeposit with it
  • emits respective channelDeposit.failure if it doesn't succeed
  • public openChannel method accepts deposit, waits for the respective events, and notify users through an onChange callback
  • adapt dApp to use above param and callback instead of slow separate open + deposit calls

With this, open + deposit can complete and be confirmed as soon as N + 1 blocks, while the previous schema required 2 * (N + 1) blocks, where N is the number of confirmation blocks.

@andrevmatos andrevmatos added sdk 🖥 optimization ⚡ Optimizations for the implementation or protocol labels Mar 4, 2020
@andrevmatos andrevmatos requested a review from nephix March 4, 2020 03:21
@andrevmatos andrevmatos self-assigned this Mar 4, 2020
@codecov
Copy link

codecov bot commented Mar 4, 2020

Codecov Report

Merging #1128 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1128      +/-   ##
==========================================
- Coverage   95.06%   95.01%   -0.05%     
==========================================
  Files         111      111              
  Lines        3972     4013      +41     
  Branches      807      827      +20     
==========================================
+ Hits         3776     3813      +37     
- Misses        154      157       +3     
- Partials       42       43       +1     
Flag Coverage Δ
#dapp 88.03% <50.00%> (-0.12%) ⬇️
#sdk 97.80% <95.23%> (-0.08%) ⬇️

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 e11c666...96734a4. Read the comment docs.

Copy link
Contributor

@nephix nephix left a comment

Choose a reason for hiding this comment

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

Looks good thank you

@nephix nephix merged commit f1fca18 into master Mar 4, 2020
@andrevmatos andrevmatos deleted the fix/open_channel_deposit branch March 4, 2020 12:37
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.

2 participants