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.

Sign up for GitHub

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

Jump to bottom

v2: fix transferOnchainTokens when subkey=true #3107

Merged
merged 1 commit into from
Apr 26, 2022
Merged

v2: fix transferOnchainTokens when subkey=true #3107

merged 1 commit into from
Apr 26, 2022

Conversation

andrevmatos
Copy link
Contributor

Fixes #3106

Short description
See issue and commit's body.

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. Serve dApp with subkey
  2. Get some tokens in subkey (e.g. by performing a small channel withdraw)
  3. Try to withdraw these tokens back to main account

@andrevmatos
sdk: fix transferOnchainTokens with subkey=true using main
37d5198 
This was caused because the `transact` helper function (which returns a
cold observable to perform/retry the transaction) also performs subkey
check, but we were not re-passing `options.subkey` to it on the public
interface.
@andrevmatos andrevmatos requested a review from taleldayekh April 25, 2022 20:22
@andrevmatos andrevmatos self-assigned this Apr 25, 2022
@codecov
Copy link

codecov bot commented Apr 25, 2022 •
edited
Loading

Codecov Report

Merging #3107 (37d5198) into v2 (acf3681) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##               v2    #3107      +/-   ##
==========================================
- Coverage   93.14%   93.14%   -0.01%     
==========================================
  Files         213      213              
  Lines        8591     8590       -1     
  Branches     1473     1473              
==========================================
- Hits         8002     8001       -1     
  Misses        530      530              
  Partials       59       59
Flag Coverage Δ
dapp 88.83% <ø> (ø)
dapp.unit 88.83% <ø> (ø)
sdk 95.43% <100.00%> (-0.01%) ⬇️
sdk.e2e 72.62% <0.00%> (+0.04%) ⬆️
sdk.integration 79.51% <0.00%> (+0.01%) ⬆️
sdk.unit 49.37% <100.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
raiden-ts/src/services/utils.ts 96.29% <ø> (ø)
raiden-ts/src/raiden.ts 94.33% <100.00%> (-0.02%) ⬇️

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 acf3681...37d5198. Read the comment docs.

taleldayekh
taleldayekh approved these changes Apr 26, 2022
View reviewed changes
Copy link
Contributor

@taleldayekh taleldayekh left a comment

Choose a reason for hiding this comment

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

Go for it, thx 👍🏼

@andrevmatos andrevmatos merged commit cc9c9ba into v2 Apr 26, 2022
@andrevmatos andrevmatos deleted the fix/sub_withdraw branch April 26, 2022 13:15
@andrevmatos andrevmatos mentioned this pull request May 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taleldayekh taleldayekh taleldayekh approved these changes

Assignees

@andrevmatos andrevmatos

Labels
sdk 🖥
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrevmatos @taleldayekh