-
Notifications
You must be signed in to change notification settings - Fork 22
refactor!(stump): Move updated data to its own function #81
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
base: main
Are you sure you want to change the base?
Conversation
Before this change, `modify` would return the data needed to update a proof for the new block. This requires additional internal computation and extra allocations. During IBD we may never use this, since proof update is only meant for updating a few blocks worth of changes. Now there's a method specific to pull the modify_data, and `modify` itself will only return a new Stump. When refactoring the addition function, I've modified it a little to allow a smarter one with a very nice property: it can pretend that it added a node, but actually represent it as deleted. The goal here is to do a Swift Sync-style protocol where you don't need deletions. If a txout is already spent, you give an empty hash (BitcoinNodeHash::empty()). This will be exactly equivalent as giving this txo's hash and later on calling delete for with this txo's hash. However, it does this with only one call to modify and doesn't require proofs to achieve that. This is an API breaking change, as modify now only returns one parameter, otherwise the behavior stays unchanged.
8eb9f98 to
bfd09ed
Compare
| /// and txos to be removed, along with it's proof. Either may be | ||
| /// empty. | ||
| /// | ||
| /// ## Adding stoxs to the stump |
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.
stoxs is a typo right ?
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.
No. Spent transaction outputs
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.
wouldnt it be stxo ?
| /// assert!(s.roots.contains(&utxos[0])); | ||
| /// assert!(!s.roots.contains(&utxos[1])); // The empty hash won't be in the roots |
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.
So if i got this correctly, you can pass a BitcoinNodeHash::empty() to tell the stump that this UTXO is spent in the future, making the acc not even adding it as a leave right ?
If im correct i would prefer to assert that the num leaves keeps being 1 and the roots num are 1.
correct me if im mistaken but, the roots in that case, if the acc included the other spent UTXO, would never be the spent UTXO, so the check
/// assert!(!s.roots.contains(&utxos[1])); // The empty hash won't be in the roots
doesnt make sense ?
That feature is already proven when you assert that the first utxo became the root right ?
| Ok(new_stump) | ||
| } | ||
|
|
||
| /// Utreexo is a dynamic accumulator, meaning that leaves can be added and removed. |
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 documentation is good but WYT about a short description on what get_updated_data does before ?
Before this change,
modifywould return the data needed to update a proof for the new block. This requires additional internal computation and extra allocations. During IBD we may never use this, since proof update is only meant for updating a few blocks worth of changes. Now there's a method specific to pull the modify_data, andmodifyitself will only return a new Stump.When refactoring the addition function, I've modified it a little to allow a smarter one with a very nice property: it can pretend that it added a node, but actually represent it as deleted. The goal here is to do a Swift Sync-style protocol where you don't need deletions.
If a txout is already spent, you give an empty hash (BitcoinNodeHash::empty()). This will be exactly equivalent as giving this txo's hash and later on calling delete for with this txo's hash. However, it does this with only one call to modify and doesn't require proofs to achieve that.
This is an API breaking change, as modify now only returns one parameter, otherwise the behavior stays unchanged.