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

Added CHUNK_FILL_COST #6983

Open
wants to merge 6 commits into
base: feature/verkle
Choose a base branch
from
Open

Conversation

yerke26
Copy link
Contributor

@yerke26 yerke26 commented May 6, 2024

Closes #6961

Changes

  • CHUNK_FILL_COST in VerkleExecutionWitness

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Optional. Remove if not applicable.

Documentation

Requires documentation update

  • Yes
  • No

If yes, link the PR to the docs update or the issue with the details labeled docs. Remove if not applicable.

Requires explanation in Release Notes

  • Yes
  • No

If yes, fill in the details here. Remove if not applicable.

Remarks

Optional. Remove if not applicable.

@@ -52,6 +52,11 @@ public class VerkleWorldState : IWorldState
// // False positives would be problematic as the code _must_ be persisted
private readonly LruKeyCache<Hash256> _codeInsertFilter = new(2048, "Code Insert Filter");

public byte[]? GetValue(Hash256 key)
Copy link
Contributor

Choose a reason for hiding this comment

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

here we only need this to check if the value is present or not - so we dont need to actually return the value
you can just return true and false and have a funciton like ValuePresentInTree
Also, you can add similar functions in the tree also, to just check presence - this will be much faster then finding and returning the value

@@ -136,7 +136,8 @@ protected virtual TransactionResult Execute(Transaction tx, in BlockExecutionCon
if (commit) WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullTxTracer.Instance);

// declare the execution witness to collect witness and also charge gas
IExecutionWitness executionWitness = spec.IsVerkleTreeEipEnabled ? new VerkleExecWitness(LogManager) : new NoExecWitness();
var verkleWorldState = WorldState as VerkleWorldState;
Copy link
Contributor

Choose a reason for hiding this comment

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

we need some kind of error handling here or inside VerkleExecWitness class.
make the input nullable and then check in the constructor, if the value is null - throw an error

@tanishqjasoria tanishqjasoria mentioned this pull request May 13, 2024
15 tasks
@tanishqjasoria tanishqjasoria linked an issue May 15, 2024 that may be closed by this pull request
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.

implement the CHUNK_FILL_COST for verkle witness
2 participants