Because 0 is the zero (default) value for uint64, if it is valid to be used as a nonce, it becomes difficult to distinguish the scenario where sender did not set a nonce from one where they explicitly set it to 0.
I'm not confident whether the ability to make this distinction matters now or has the potential to later but was following a feeling.
Summary generated by Reviewpad on 11 Jul 23 11:56 UTC
This pull request includes the following changes:
In the p2p/module.go file, the handlePocketEnvelope function now checks if poktEnvelope.Nonce is zero and returns an error if it is. The error message includes the hex-encoded nonce value.
In the p2p/module_test.go file, new test cases have been added to test the handling of an invalid nonce.
In the p2p/types/errors.go file, a new error variable ErrInvalidNonce has been added to represent an invalid nonce value.
In the shared/crypto/rand.go file, a check has been added to ensure that the generated nonce value is not zero. If it is zero, the function recursively calls itself to generate a new nonce.
Issue
N/A; observation made while working on #732.
Type of change
Please mark the relevant option(s):
[ ] New feature, functionality or library
[x] Bug fix
[ ] Code health or cleanup
[ ] Major breaking change
[ ] Documentation
[ ] Other
List of changes
Adds ErrInvalidNonce P2P error type
Ensures the P2P message handler rejects the zero value Nonce (uint64(0)) is invalid
Ensures the GetNonce function never returns the zero value
Testing
[ ] make develop_test; if any code changes were made
[ ] make test_e2e on k8s LocalNet; if any code changes were made
[ ] e2e-devnet-test passes tests on DevNet; if any code was changed
Description
Because
0is the zero (default) value foruint64, if it is valid to be used as a nonce, it becomes difficult to distinguish the scenario where sender did not set a nonce from one where they explicitly set it to0.I'm not confident whether the ability to make this distinction matters now or has the potential to later but was following a feeling.
Summary generated by Reviewpad on 11 Jul 23 11:56 UTC
This pull request includes the following changes:
p2p/module.gofile, thehandlePocketEnvelopefunction now checks ifpoktEnvelope.Nonceis zero and returns an error if it is. The error message includes the hex-encoded nonce value.p2p/module_test.gofile, new test cases have been added to test the handling of an invalid nonce.p2p/types/errors.gofile, a new error variableErrInvalidNoncehas been added to represent an invalid nonce value.shared/crypto/rand.gofile, a check has been added to ensure that the generated nonce value is not zero. If it is zero, the function recursively calls itself to generate a new nonce.Issue
N/A; observation made while working on #732.
Type of change
Please mark the relevant option(s):
List of changes
ErrInvalidNonceP2P error typeNonce(uint64(0)) is invalidGetNoncefunction never returns the zero valueTesting
make develop_test; if any code changes were mademake test_e2eon k8s LocalNet; if any code changes were madee2e-devnet-testpasses tests on DevNet; if any code was changedRequired Checklist
godocformat comments on touched members (see: tip.golang.org/doc/comment)If Applicable Checklist
shared/docs/*if I updatedshared/*README(s)