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

Next #2822

Merged
merged 35 commits into from
Jun 16, 2021
Merged

Next #2822

merged 35 commits into from
Jun 16, 2021

Conversation

andrevmatos
Copy link
Contributor

Fixes #

Short description

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)

@codecov
Copy link

codecov bot commented Jun 15, 2021

Codecov Report

Merging #2822 (3837b94) into master (fa3089f) will increase coverage by 0.08%.
The diff coverage is 97.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2822      +/-   ##
==========================================
+ Coverage   93.51%   93.60%   +0.08%     
==========================================
  Files         111      110       -1     
  Lines        6001     6036      +35     
  Branches     1086     1093       +7     
==========================================
+ Hits         5612     5650      +38     
+ Misses        335      331       -4     
- Partials       54       55       +1     
Flag Coverage Δ
dapp 80.64% <ø> (ø)
dapp.unit 80.64% <ø> (ø)
sdk 95.78% <97.43%> (+0.08%) ⬆️
sdk.e2e 74.72% <91.88%> (+0.07%) ⬆️
sdk.integration 78.48% <97.43%> (+0.04%) ⬆️
sdk.unit 50.27% <32.47%> (+0.18%) ⬆️

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

Impacted Files Coverage Δ
raiden-ts/src/config.ts 94.59% <ø> (-0.28%) ⬇️
raiden-ts/src/raiden.ts 94.43% <ø> (ø)
raiden-ts/src/messages/types.ts 97.33% <91.30%> (-2.67%) ⬇️
raiden-ts/src/transfers/mediate/epics.ts 95.74% <92.00%> (-0.49%) ⬇️
raiden-ts/src/services/utils.ts 97.77% <96.66%> (+9.54%) ⬆️
raiden-ts/src/transport/epics/messages.ts 94.51% <96.77%> (+0.10%) ⬆️
raiden-ts/src/messages/actions.ts 100.00% <100.00%> (ø)
raiden-ts/src/messages/utils.ts 99.10% <100.00%> (+0.04%) ⬆️
raiden-ts/src/services/epics/helpers.ts 94.44% <100.00%> (+0.26%) ⬆️
raiden-ts/src/services/types.ts 100.00% <100.00%> (ø)
... and 16 more

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 fa3089f...3837b94. Read the comment docs.

@andrevmatos andrevmatos requested a review from palango June 15, 2021 17:05
@andrevmatos andrevmatos self-assigned this Jun 15, 2021
@andrevmatos andrevmatos added next Issues/PRs targeting updates of Raiden contracts or protocol breaking 💔 For breaking changes in the protocol or services labels Jun 15, 2021
andrevmatos and others added 22 commits June 15, 2021 14:11
This removes completely the requirement to having access to peer's
presence directly from the matrix server, and instead replace it with
the requirement for the PFS to know (already required for mediated
transfers) and provide that presence (implemented in `next` RSB).
Instead of searchUserDirectory, we request the PFS
`/address/.../metadata` endpoint for its view of latest peer's presence.
Now that presence is fetched from PFS, we don't need to join discovery
global room anymore, and since PFS and MS rooms were removed previously,
we can clear the utility and helpers which supported this.
Also fix some typos
and validate peer's displayname signature coming from service
Services now return a new `address_metadata` field which maps addresses
to their respective metadata/presence object. We need to handle this,
and pass it in a transfer's metadata. Besides this, mediated transfers
should try to use this mapping in the incoming transfer's metadata to
optimize not having to request metadata from service to send the output
message. This usage, despite validated by signature, should be
constrained to this transfer only (for now).
andrevmatos and others added 11 commits June 15, 2021 14:11
Processed, SecretRequest, SecretReveal and Unlock messages
Also, don't use messageSend's userId on retries:
If this message had to be retried, maybe the userId isn't valid anymore,
so we remove it on retries, to force transport to re-fetch this peer's
presence/metadata, or mind its own caching mechanism as needed.
So, PC mediators assume they're the 1st address in route, and their
partner 2nd, and trim it when forwarding the transfer. We comply here
until they can handle passing whole metadata unchanged.
PC signs metadata with checksummed addresses, but send them all
lowercase-serialized. We were also decoding addresses checksummed, but
`address_metadata`, being a dict, was being exempted from this change.
This is just a workaround: proper fix is to perfectly sign what is
received/sent (passthrough), and being able to handle receiving both.
Copy link
Contributor

@weilbith weilbith left a comment

Choose a reason for hiding this comment

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

Basically everything has been already review by PRs targeting the next branch. I just took a quick look on the last commits. Looks all good to me. So I don't see a reason why we should not merge this to master. So the final testing before the mainnet release can start.

@weilbith weilbith merged commit cd484a6 into master Jun 16, 2021
@weilbith weilbith deleted the next branch June 16, 2021 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking 💔 For breaking changes in the protocol or services next Issues/PRs targeting updates of Raiden contracts or protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants