-
Notifications
You must be signed in to change notification settings - Fork 334
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
Add signature v2 format #983
Add signature v2 format #983
Conversation
why not just up the network version itself? edit: then use the version to decide on the parse method |
That wouldn't be backward compatible - new dendrites cannot talk to old axons (since old axons expect the old format). In this way there can be a transition period in which new dendrites can speak with old axons, and old axons allow traffic from old dendrites. In some future version, the |
I'm not suggesting you get rid of this. I'm suggesting you use the wire protocol version to do this instead of adding metadata |
So I've ended up doing this based on the advertised version on chain. The dendrite/receptor decides based on the endpoint version which signature to use. Note that this is a bit awkward right now - we cannot use version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I think we can safely begin the review process on this PR. @eduardogr I would like your take as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job! I like the new signature function. There are a few things that we need to fix before merging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Nice job!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,
(commented already, i miss a couple of unit tests but everything good)
Thanks for this!
@adriansmares Merging this PR today, it will be released in the next version |
Summary
References #976
This PR introduces a new signature format for requests, in order to avoid situations in which validator requests are fulfilled by nodes which are not targeted by the RPC.
This PR is based on #976 in order to avoid merge conflicts - only the last two commits are relevant here.
Changes
v2
signature format, which also takes into account the targetaxon
hotkey. Thev2
signature ensures that the signature cannot be faked by an intermediary that is not a validator.