predicate results: alternate approach (similar to master) #679
Conversation
darioush
changed the title
params/hooks_libevm.go
Outdated
| @@ -192,6 +196,6 @@ func (b *BlockContext) Timestamp() uint64 { | |||
| return b.time | |||
| } | |||
|
|
|||
| func (b *BlockContext) GetPredicateResultsBytes() []byte { | |||
| return b.predicateResultsBytes | |||
| func (b *BlockContext) GetPredicateResults(txHash common.Hash, precompileAddress common.Address) []byte { | |||
There was a problem hiding this comment.
should this return predicate.Results?
There was a problem hiding this comment.
or keep the byte in name?
There was a problem hiding this comment.
This is how it is in master so I left it like that for now.
params/predicate_bytes.go
Outdated
There was a problem hiding this comment.
what is the cyclic import that prevents us to have these in predicate pkg?
There was a problem hiding this comment.
predicates imports params which needs to import predicates again to parse the predicates to make the block context for the precompiles.
I'm somewhat hopeful that we can fix this by finding a better location for the libevm hooks other than "params", which we can address next.
also I mildly dislike that the predicates package needs to know about the length of the dynamic fee window.
There was a problem hiding this comment.
If the dynamic fee window is the only reason we're importing params package I think it's fine to copy/move it to predicate pkg and move these util functions there
There was a problem hiding this comment.
params in general is a really bad package for constants, and in our case it's even worse.
params/hooks_libevm.go
Outdated
| predicateResults, err = predicate.ParseResults(predicateResultsBytes) | ||
| if err != nil { | ||
| panic(err) // Should never happen, as results are already validated in block validation | ||
| } | ||
| } | ||
| accessableState := accessableState{ | ||
| env: env, | ||
| blockContext: &BlockContext{ |
There was a problem hiding this comment.
why we cannot use vm.BlockContext directly from env? It should already have a parsed results (and other similar functions to implement required interface)
There was a problem hiding this comment.
perhaps you are looking at the subnet-evm or coreth source code?
as the vm.BlockContext here would be from the upstream libevm code here, which we modified to include the full header so we can access the extra bits (aka predicate results) here and pass it to the precompiles.
Why this should be merged
This PR is an alternative to #675
This PR makes the precompile interface as it is in master, i.e. with
GetPredicateResults(txHash common.Hash, precompileAddress common.Address)How this works
This PR removes the dependency from predicators package to the params package by:
After this dependency is removed, the parsing functions from predicates package can be used in params.
How this was tested
CI