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

fix: add proof of absence for chunks when contract creation reverts #331

Conversation

gballet
Copy link
Owner

@gballet gballet commented Jan 16, 2024

Adding contract chunks to the witness only occurred when the creation transaction was a success.

The question that arises is: do we need to add the address to the witness, knowing that it's not created? At first, I thought that this was the case, but actually since the creation isn't executed, it should not be the case. This needs to be though through further.

@gballet gballet added this to the verkle-gen-devnet-3 milestone Jan 16, 2024
err = ErrOutOfGas
if evm.chainRules.IsPrague {
if err == nil || err == ErrExecutionReverted {
if !contract.UseGas(evm.Accesses.TouchAndChargeContractCreateCompleted(address.Bytes()[:])) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

actually in the context of a reverted execution, the values should be nil and not the destination values. Let's first confirm that this is indeed a bug that needs fixing, I'm not so sure anymore.

@gballet gballet marked this pull request as ready for review January 26, 2024 10:35
@gballet
Copy link
Owner Author

gballet commented Jan 26, 2024

After thinking it through, it appears that this makes no sense: the contract isn't added to the state, since its creation reverted. So there wouldn't be any change to the tree, so no need to add it to the witness: the stateless client will not try to access the tree at this location either.

@gballet gballet closed this Jan 26, 2024
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.

1 participant