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

send encrypted secret #7196

Conversation

netcriptus
Copy link
Contributor

Allow encrypted secret to be sent on LockedTransfer's metadata

Fixes: #7191

@auto-assign auto-assign bot requested a review from fredo June 29, 2021 15:21
raiden/transfer/utils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@fredo fredo left a comment

Choose a reason for hiding this comment

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

LGTM in general, good job!

Once you have implemented the encryption with the correct public key we are good to go.

2 things to take care/notice:

  1. Somewhere deep in my memory I remember that it was not allowed to use an empty (b'0') bytestring as the secret. We should ask some Raiden OGs in the stand up.
  2. Some tests that encryption/decryption works would be good.

@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch from d11e81b to bcf1c55 Compare July 1, 2021 15:33
@netcriptus netcriptus requested a review from fredo July 1, 2021 15:42
raiden/messages/metadata.py Outdated Show resolved Hide resolved
@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch from 89b499a to 82ab477 Compare July 2, 2021 13:30
@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch 2 times, most recently from 619deb1 to d5e5de0 Compare July 7, 2021 13:20
@istankovic
Copy link
Contributor

@netcriptus nitpick: it would be nice to squash some of the changes (e.g. the pdb stuff, hex encode fix) here and split some others
(like the addition of dependencies in the "refactor tests" commit 26012b3).

@netcriptus
Copy link
Contributor Author

@netcriptus nitpick: it would be nice to squash some of the changes (e.g. the pdb stuff, hex encode fix) here and split some others
(like the addition of dependencies in the "refactor tests" commit 26012b3).

Will most definitely do it, but sadly I'm not done with this yet. I'm working on more tests. I can promise the squash will come later.

Copy link
Contributor

@ezdac ezdac left a comment

Choose a reason for hiding this comment

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

Some things that need to be addressed.
Also we have to sync with the tsRaiden team again - (the PR could still be merged first)

raiden/tests/unit/serialized_messages/LockedTransfer.json Outdated Show resolved Hide resolved
raiden/tests/unit/serialized_messages/LockedTransfer.json Outdated Show resolved Hide resolved
raiden/messages/metadata.py Outdated Show resolved Hide resolved
@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch 3 times, most recently from 682dbbe to b1aba04 Compare July 12, 2021 14:12
@netcriptus netcriptus requested review from ezdac and fredo July 12, 2021 14:15
@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch 2 times, most recently from 6454805 to 5bcc4fa Compare July 12, 2021 15:22
Copy link
Contributor

@fredo fredo 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 to me! thanks

Copy link
Contributor

@ezdac ezdac left a comment

Choose a reason for hiding this comment

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

I discovered a flaw in the serialize-missing mechanism. I'd still rather fix the underlying issue, but possibly we can do that at a later time and merge the PR?
Still, let's discuss briefly in private before merging

raiden/messages/metadata.py Outdated Show resolved Hide resolved
@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch from 5bcc4fa to 2d13a7f Compare July 13, 2021 08:57
@netcriptus netcriptus force-pushed the feature/send-encrypted-secret-7191 branch from 2d13a7f to 0098ff9 Compare July 13, 2021 09:02
@netcriptus netcriptus merged commit 704698d into raiden-network:develop Jul 13, 2021
@netcriptus netcriptus deleted the feature/send-encrypted-secret-7191 branch July 13, 2021 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initiator sends encrypted secret in metadata
4 participants