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: inaccuracies in delegate action errors #496

Merged
merged 2 commits into from
Aug 4, 2023

Conversation

jakmeier
Copy link
Contributor

@jakmeier jakmeier commented Aug 3, 2023

While updating the specs for delegate actions, I noticed that there are two small mistakes in the error description of the NEP. Or I guess it would be more accurate to say the nearcore implementation is not compliant with the NEP.

  1. DelegateActionCantContainNestedOne is impossible to happen because the chosen serialization format does not allow nesting in the first place.
  2. The name for DelegateActionSenderDoesNotMatchReceiver somehow managed to get even longer in the real implementation and now spells DelegateActionSenderDoesNotMatchTxReceiver

I know this isn't ideal. But since this is now de-facto part of the protocol, I suggest we update the NEP to reflect it. Alternatively, we could change it in nearcore. This would be a breaking change in some primitives crates, so I tend towards the first option.

While updating the specs for delegate actions, I noticed that there are
two small mistakes in the error description of the NEP.
Or I guess it would be more accurate to say the nearcore implementation
is not compliant with the NEP.

1) `DelegateActionCantContainNestedOne` is impossible to happen because
the chosen serialization format does not allow nesting
in the first place.
2) The name for `DelegateActionSenderDoesNotMatchReceiver` somehow
managed to get even longer in the real implementation
and now spells `DelegateActionSenderDoesNotMatchTxReceiver`

I know this isn't ideal. But since this is now de-facto part of the
protocol, I suggest we update the NEP to reflect it.
Alternatively, we could change it in nearcore, which
would be a breaking change in some primitives crates.
@jakmeier jakmeier requested a review from a team as a code owner August 3, 2023 10:06
Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

@jakmeier Good catch! Thanks for addressing it!

@frol frol merged commit 9cb94e0 into near:master Aug 4, 2023
3 checks passed
@frol frol added S-approved A NEP that was approved by a working group. A-Nomicon-Changes An update to the Nomicon documentation. labels Aug 4, 2023
@jakmeier jakmeier deleted the fix_nep366_errors branch August 4, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Nomicon-Changes An update to the Nomicon documentation. S-approved A NEP that was approved by a working group.
Projects
Status: APPROVED FIXES
Development

Successfully merging this pull request may close these issues.

2 participants