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

Fix revealTimeout on testnets #3118

Merged
merged 2 commits into from
May 5, 2022
Merged

Conversation

andrevmatos
Copy link
Contributor

Fixes #

Short description
This fixes a small issue identified on Arbitrum release (v3) on testnets (e.g. arbitrum-rinkeby), where settleTimeout in deployed contracts (180 seconds) is smaller than default revealTimeout (600 seconds), making every transfer fail with expires too soon error.

It also includes some simplification of conditionals in circleCI config, preparing for next [patch] release.

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)

  1. Transfers work out-of-the-box on contracts with settleTimeout<600

@andrevmatos andrevmatos added sdk 🖥 infrastructure 🚧 Tests, CI, and general project infrastructure Rollups ⭕ labels May 4, 2022
@andrevmatos andrevmatos self-assigned this May 4, 2022
When fetching contract's settleTimeout, if revealTimeout is too large,
we set it explicitly to a sane default (settleTimeout / 2), instead of
leaving it there and failing transfer's requests until the user pass a
suitable config.revealTimeout.
Also, for that, we relax a little the transfer `expiration` constraint
to `<settleTimeout`, so half of that will still work with expiryFactor=2
@andrevmatos andrevmatos force-pushed the fix/reveal_timeout_testnets branch from cdf3f66 to 4568bf2 Compare May 4, 2022 15:47
@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #3118 (4568bf2) into master (4fd58bc) will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #3118      +/-   ##
==========================================
+ Coverage   93.92%   93.94%   +0.01%     
==========================================
  Files         213      213              
  Lines        7404     7411       +7     
  Branches     1244     1247       +3     
==========================================
+ Hits         6954     6962       +8     
+ Misses        416      415       -1     
  Partials       34       34              
Flag Coverage Δ
dapp 89.89% <ø> (ø)
dapp.unit 89.89% <ø> (ø)
sdk 95.23% <66.66%> (+0.02%) ⬆️
sdk.e2e 71.88% <66.66%> (-0.06%) ⬇️
sdk.integration 79.69% <66.66%> (+0.04%) ⬆️
sdk.unit 49.06% <8.33%> (-0.07%) ⬇️

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

Impacted Files Coverage Δ
raiden-ts/src/channels/epics/block.ts 95.34% <60.00%> (-4.66%) ⬇️
raiden-ts/src/transfers/epics/locked.ts 96.16% <100.00%> (ø)
raiden-ts/src/transport/epics/webrtc.ts 94.59% <0.00%> (+1.93%) ⬆️

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 4fd58bc...4568bf2. Read the comment docs.

@andrevmatos andrevmatos merged commit 3e69251 into master May 5, 2022
@andrevmatos andrevmatos deleted the fix/reveal_timeout_testnets branch May 5, 2022 10:48
@karlb karlb mentioned this pull request Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure 🚧 Tests, CI, and general project infrastructure Rollups ⭕ sdk 🖥
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants