-
Notifications
You must be signed in to change notification settings - Fork 990
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
Remove Merkle Proof Generation from Foreign API get_outputs
#3792
Merged
yeastplume
merged 4 commits into
mimblewimble:master
from
yeastplume:remove_merkle_proof_master
Jun 18, 2024
Merged
Remove Merkle Proof Generation from Foreign API get_outputs
#3792
yeastplume
merged 4 commits into
mimblewimble:master
from
yeastplume:remove_merkle_proof_master
Jun 18, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yeastplume
changed the title
Remove Merkle Generation from Foreign API
Remove Merkle Proof Generation from Foreign API Jun 17, 2024
get_outputs
get_outputs
tromp
approved these changes
Jun 18, 2024
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.
Looks fine to me.
yeastplume
added a commit
to yeastplume/grin
that referenced
this pull request
Jun 19, 2024
…wimble#3792) * Update versioning on master to 5.4.0-alpha.0 * Remove merkle proof generation from foreign API
bayk
added a commit
to mwcproject/mwc-node
that referenced
this pull request
Jun 29, 2024
…_outputs` (mimblewimble#3792) (mimblewimble#3793) MWC - Merkle Proof is not removed, it is optional requested data, let's not break API
seems fine |
yeastplume
added a commit
that referenced
this pull request
Sep 16, 2024
* Update grin_secp to 0.7.14 (#3788) * Update versioning on master to 5.4.0-alpha.0 * Update versioning on master to 5.4.0-alpha.0 (#3789) * Remove Merkle Proof Generation from Foreign API `get_outputs` (#3792) * Update versioning on master to 5.4.0-alpha.0 * Remove merkle proof generation from foreign API * Rust 1.80+ fixes & accumulated warning cleanup (#3796) * Update versioning on master to 5.4.0-alpha.0 * updates for 1.80 and other accumulated warnings * further warning cleanups * move dead code tag to function defn rather than module * Chain type field (#3790) * Adding chain type field into get_status rpc * formatting * update version for next build --------- Co-authored-by: aglkm <39521015+aglkm@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As evidenced by #3791, Merkle proof generation is an experimental feature that currently isn't being used but also has the potential to grind nodes to a halt, as a request for a proof for an early output requires the chain to rewind to that position. This is an obvious problem for archival nodes, but also an issue on non-archival nodes due to the ability to request the genesis coinbase output and force a rewind of the entire chain.
This functionality had been turned off for the
get_block
function, but for some reason remained within theget_outputs
function.#3487
Note there's no known use for these Merkle proofs as present (PIBD uses a form of them, but has its own code for proof generation and never requires a rewind due to it only operating on tree states at the horizon). This PR modifies the foreign API call to ignore the
include_merkle_proof
field in theget_outputs
foreign api, while leaving the parameter in place to avoid breaking existing installations. Instructions to turn it back on are embedded.Once merged into master, will cherry-pick to 5.3.x branch and rebuild.