-
Notifications
You must be signed in to change notification settings - Fork 220
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(core)!: add missing consensus encoding length byte rangeproof & covenants #3730
fix(core)!: add missing consensus encoding length byte rangeproof & covenants #3730
Conversation
20f8c60
to
1b531e4
Compare
1b531e4
to
efa7bb5
Compare
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.
This looks good. Made a comment or two.
We can remove all the places where we split the comsig's and then encode them as this pr now adds an implementation to implement them directly.
I also think we should use consensus encoding everywhere in the hashes and challenges as this is committed to on the blockchain, either in a MMR root, or just straight up in the output/input. And if we want another implementation to be able to verify this it needs to be able to decode/encode it.
base_layer/core/src/transactions/transaction/transaction_input.rs
Outdated
Show resolved
Hide resolved
a864af3
to
a44fd56
Compare
a44fd56
to
84d7b8e
Compare
Description
Motivation and Context
Rangeproof is not fixed size and so requires a length varint.
It is important to ensure that a remote peer cannot cause unbounded allocation, so various length checks are implemented.
The wallet previously only ever stored a 0-byte covenant which is now invalid encoding.
Potential fix for WriteZero, ensuring that all rpc messages are non-zero in length (rare, but possible).
How Has This Been Tested?
Existing tests updated, manually - mined blocks on dibbler