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

[feat] Utreexo: adding new item to the accumulator #167 #190

Merged

Conversation

Jeanmichel7
Copy link
Collaborator

@Jeanmichel7 Jeanmichel7 commented Sep 13, 2024

Copy link

vercel bot commented Sep 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
raito ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 18, 2024 1:57pm

@Jeanmichel7 Jeanmichel7 changed the title test [feat] Utreexo: adding new item to the accumulator #167 Sep 13, 2024
@Jeanmichel7
Copy link
Collaborator Author

ok because we have to remove it from the cache when we delete an utxo

@m-kus
Copy link
Collaborator

m-kus commented Sep 13, 2024

@Jeanmichel7 cache is a separate task #164 you don't need to implement that

@Jeanmichel7
Copy link
Collaborator Author

Jeanmichel7 commented Sep 15, 2024

@m-kus @maciejka @TAdev0 I wonder if the type we want to add to the utreexo accumulator isn't just the hash of the output (without the cached field)?
It's when you go through the array of outputs of a transaction, that you add the leaves to the available utxo set.
So we don't need to provide an OutPoint but a TxOut to the add function?

The OutPoint is only useful for validate and delete the utxos available in the accumulator provided in a transaction's input.

Did I understand correctly?

I didn't know how to test the logic of the accumulator, so I tried to use a dict which is very close to the basic algo to compare the diff.

@m-kus
Copy link
Collaborator

m-kus commented Sep 15, 2024

We need the entire outpoint because we rely on extra fields like block height/time and iscoinbase for finality checks.

If this data is not in a leaf or cache it is unconstrained, ie you can provide arbitrary values and the program won’t be able to tell

@Jeanmichel7
Copy link
Collaborator Author

So we assume that when we add an OutPoint, the cache in data is always set to true? otherwise it could create 2hash for the same UTXO?

@m-kus
Copy link
Collaborator

m-kus commented Sep 15, 2024

It’s a bit more complex, but we’ll get there gradually. Generally we want to ensure two things:

  • All referenced outpoints are constrained (either via utreexo or cache)
  • all the referenced outpoints are spent exactly once

When we handle a transaction input there might be two cases:

  • it is cached, we need to check that it actually exists in the cache and then we remove it
  • otherwise we check that it hasn’t been spent yet, and mark for removal

For every output we construct the outpoint and either put it in the cache or mark for addition.

In the end we do batch inclusion verification/removal, the add all the new leaves.

@TAdev0
Copy link
Collaborator

TAdev0 commented Sep 16, 2024

resolves #167

doesnt work when its not in the first comment

@Jeanmichel7 can you add it on the first comment of the issue? so that it links your PR to the issue , and it will also automatically close the issue once merged

@m-kus
Copy link
Collaborator

m-kus commented Sep 18, 2024

LGTM
Some refactoring is needed (e.g. it is clear now that we can separate UtxoSet and Utreexo accumulator), but it's better to do after we merge all pending PRs

@m-kus m-kus merged commit 105c6ef into keep-starknet-strange:main Sep 18, 2024
6 checks passed
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.

[feat] Utreexo: adding new item to the accumulator
3 participants