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

ringct: remove unused range proof types and fix serialization bug #9717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeffro256
Copy link
Contributor

Remove unused Rangeproof types RangeProofBulletproof , and RangeProofMultiOutputBulletproof. In the prove code, if the RangeProofBulletproof enum was passed, then one bulletproof was created per output. If RangeProofMultiOutputBulletproof was passed, then K bulletproofs were produced, where K is the number of one bits in the count of the outputs, Neither of these are allowed by consensus:

if (rv.p.bulletproofs_plus.size() != 1)
{
LOG_PRINT_L1("Failed to parse transaction from blob, bad bulletproofs_plus size in tx " << get_transaction_hash(tx));
return false;
}
and
if (rv.p.bulletproofs.size() != 1)
{
LOG_PRINT_L1("Failed to parse transaction from blob, bad bulletproofs size in tx " << get_transaction_hash(tx));
return false;
}
.

Also fixes an actual bug with the boost serialization of type tools::wallet2::signed_tx_set. If the serialized payload has a version < 4, then the range proof type is assumed to be RangeProofBulletproof, which is incorrect.

@selsta
Copy link
Collaborator

selsta commented Jan 18, 2025

Compilation issues with some CI jobs

@jeffro256
Copy link
Contributor Author

Oops sorry I only pushed some of the commits

@jeffro256 jeffro256 force-pushed the rm_unused_range_proof_types branch from 1049280 to ff4e86d Compare January 19, 2025 05:40
@iamamyth
Copy link

Looks like there's still one failing unit test:
select_outputs.exact_unlock_block

@jeffro256
Copy link
Contributor Author

That test should be unrelated, and fails sporadically.

@iamamyth
Copy link

Agreed, unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants