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

Preserve commitment format inside transactions #8277

Merged
merged 1 commit into from
May 10, 2022
Merged

Preserve commitment format inside transactions #8277

merged 1 commit into from
May 10, 2022

Conversation

kayabaNerve
Copy link

A change to the format commitment / 8 was included with BP+, which is a minor optimization which increased complexity, spread out where crypto code is handled, and created a disparate state between transactions and disk (as they were saved in their full form thanks to a processing step inside the DB code).

This PR reverts this planned change. The blockchain_db.cpp edits may look to be further edited than just this reversion, yet that was its code pre-BP+.

Copy link
Contributor

@UkoeHB UkoeHB left a comment

Choose a reason for hiding this comment

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

LGTM. Every change here lines up 1:1 with conditionals introduced by #7170 (BP+).

@UkoeHB UkoeHB mentioned this pull request Apr 24, 2022
j-berman added a commit to j-berman/monero-lws that referenced this pull request Apr 27, 2022
Copy link
Collaborator

@moneromooo-monero moneromooo-monero left a comment

Choose a reason for hiding this comment

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

Technically one hunk's missing from the revert (const bool plus), but it doesn't seem needed for now.

It'd be nice to see the verification speed change this makes though. I don't remember what it was.

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

Successfully merging this pull request may close these issues.

4 participants