Skip to content

Fix mutable data#14

Open
exqlnet wants to merge 4 commits into
wealdtech:masterfrom
exqlnet:fix-mutable-data
Open

Fix mutable data#14
exqlnet wants to merge 4 commits into
wealdtech:masterfrom
exqlnet:fix-mutable-data

Conversation

@exqlnet
Copy link
Copy Markdown
Contributor

@exqlnet exqlnet commented Mar 13, 2024

It's safer to use copy of data while constructing the tree. Found this bug because I failed to pass NewTree test separately while go test ./... passed. So there are also some problems with unittest answer.

@exqlnet exqlnet mentioned this pull request Mar 13, 2024
Copy link
Copy Markdown
Collaborator

@mcdee mcdee left a comment

Choose a reason for hiding this comment

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

Thanks for this; a couple of requests.

Comment thread parameters.go
@@ -43,7 +43,12 @@ func (f parameterFunc) apply(p *parameters) {
// WithData sets the data for the merkle tree.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please could you add a note here that this copies the data?

Comment thread multiproof_test.go
// Test proof for all combinations of data.
var proof *MultiProof
combinations := 1<<len(test.data) - 1
combinations := 1<<len(tree.Data) - 1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are other tests that use test.data in a similar fashion, please could you update those as well?

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.

2 participants