Return an error in EncodeSignedTxn() and DecodeSignedTxn() if consensus protocol is unknown.
#2708
Conversation
tolikzinovyev
changed the title
EncodeSignedTxn() and DecodeSignedTxn() if consensus protocol is unknown.
Codecov Report
@@ Coverage Diff @@
## master #2708 +/- ##
==========================================
- Coverage 47.07% 47.05% -0.02%
==========================================
Files 349 349
Lines 55858 55864 +6
==========================================
- Hits 26294 26287 -7
- Misses 26617 26623 +6
- Partials 2947 2954 +7
Continue to review full report at Codecov.
|
node/indexer/indexer_test.go
Outdated
| const genesisID string = "foo" | ||
|
|
||
| var genesisHash crypto.Digest |
There was a problem hiding this comment.
rename these to testGenesisID and testGenesisHash.
The testGenesisHash can be defined here as
var testGenesisHash = crypto.Digest{0x1, 0x2, 0x3..}which is abit cleaner than defining it here and initializing it in init
node/topAccountListener_test.go
Outdated
| blk.BlockHeader.GenesisID = "foo" | ||
| blk.BlockHeader.GenesisHash[0] = 3 |
There was a problem hiding this comment.
I'm not sure why you need to make the GenesisID/GenesisHash.. but there is nothing wrong with these changes.
There was a problem hiding this comment.
it would generally be better if you were to pick a genesisHash that is really pseudo-random, i.e. crypto.Hash([]byte{1,2,3,4})
There was a problem hiding this comment.
GenesisID and GenesisHash must the same in transactions and block header. Why is pseudo-random better?
There was a problem hiding this comment.
a true random means that the result of the test might be random.. and we don't want that.
filling up only selected bytes doesn't guarantee that the comparison was testing all the bytes.
(i.e. both me and you knows it does.. but we want to ensure we test that )
tsachiherman
left a comment
tsachiherman
left a comment
There was a problem hiding this comment.
looks good; one request for renaming global test variable names.
3 participants
No description provided.