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

predicate results: alternate approach (similar to master) #679

Merged
merged 4 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ type ChainContext interface {

// NewEVMBlockContext creates a new context for use in the EVM.
func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common.Address) vm.BlockContext {
predicateBytes := predicate.GetPredicateResultBytes(header.Extra)
predicateBytes := params.GetPredicateResultBytes(header.Extra)
if len(predicateBytes) == 0 {
return newEVMBlockContext(header, chain, author, nil)
}
Expand All @@ -129,7 +129,7 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common
func NewEVMBlockContextWithPredicateResults(header *types.Header, chain ChainContext, author *common.Address, predicateBytes []byte) vm.BlockContext {
extra := bytes.Clone(header.Extra)
if len(predicateBytes) > 0 {
extra = predicate.SetPredicateResultBytes(extra, predicateBytes)
extra = params.SetPredicateResultBytes(extra, predicateBytes)
}
return newEVMBlockContext(header, chain, author, extra)
}
Expand Down
3 changes: 2 additions & 1 deletion core/extstate/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ type StateDB struct {
}

func (s *StateDB) Prepare(rules params.Rules, sender, coinbase common.Address, dst *common.Address, precompiles []common.Address, list types.AccessList) {
s.predicateStorageSlots = predicate.PreparePredicateStorageSlots(rules, list)
rulesExtra := params.GetRulesExtra(rules)
s.predicateStorageSlots = predicate.PreparePredicateStorageSlots(rulesExtra, list)
s.VmStateDB.Prepare(rules, sender, coinbase, dst, precompiles, list)
}

Expand Down
5 changes: 3 additions & 2 deletions core/predicate_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ func CheckPredicates(rules params.Rules, predicateContext *precompileconfig.Pred
return nil, fmt.Errorf("%w for predicate verification (%d) < intrinsic gas (%d)", ErrIntrinsicGas, tx.Gas(), intrinsicGas)
}

rulesExtra := params.GetRulesExtra(rules)
predicateResults := make(map[common.Address][]byte)
// Short circuit early if there are no precompile predicates to verify
if !params.GetRulesExtra(rules).PredicatersExist() {
if !rulesExtra.PredicatersExist() {
return predicateResults, nil
}

// Prepare the predicate storage slots from the transaction's access list
predicateArguments := predicate.PreparePredicateStorageSlots(rules, tx.AccessList())
predicateArguments := predicate.PreparePredicateStorageSlots(rulesExtra, tx.AccessList())

// If there are no predicates to verify, return early and skip requiring the proposervm block
// context to be populated.
Expand Down
5 changes: 1 addition & 4 deletions eth/tracers/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"github.com/ava-labs/coreth/core/types"
"github.com/ava-labs/coreth/internal/ethapi"
"github.com/ava-labs/coreth/params"
"github.com/ava-labs/coreth/predicate"
"github.com/ava-labs/coreth/rpc"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
Expand Down Expand Up @@ -967,9 +966,7 @@ func (api *API) TraceCall(ctx context.Context, args ethapi.TransactionArgs, bloc
config.BlockOverrides.Apply(&vmctx)
// Apply all relevant upgrades from [originalTime] to the block time set in the override.
// Should be applied before the state overrides.
predicateBytes := predicate.GetPredicateResultBytes(vmctx.Header.Extra)
blockContext := params.NewBlockContext(vmctx.BlockNumber, vmctx.Time, predicateBytes)
err = core.ApplyUpgrades(api.backend.ChainConfig(), &originalTime, blockContext, statedb)
err = core.ApplyUpgrades(api.backend.ChainConfig(), &originalTime, block, statedb)
if err != nil {
return nil, err
}
Expand Down
51 changes: 25 additions & 26 deletions params/hooks_libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/ava-labs/coreth/precompile/contract"
"github.com/ava-labs/coreth/precompile/modules"
"github.com/ava-labs/coreth/precompile/precompileconfig"
"github.com/ava-labs/coreth/predicate"
"github.com/ava-labs/coreth/vmerrs"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/vm"
Expand Down Expand Up @@ -106,16 +107,19 @@ func makePrecompile(contract contract.StatefulPrecompiledContract) libevm.Precom
if err != nil {
panic(err) // Should never happen
}
var predicateResultsBytes []byte
if len(header.Extra) >= DynamicFeeExtraDataSize {
predicateResultsBytes = header.Extra[DynamicFeeExtraDataSize:]
var predicateResults *predicate.Results
if predicateResultsBytes := GetPredicateResultBytes(header.Extra); len(predicateResultsBytes) > 0 {
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{
number: env.BlockNumber(),
time: env.BlockTime(),
predicateResultsBytes: predicateResultsBytes,
blockContext: &precompileBlockContext{
number: env.BlockNumber(),
time: env.BlockTime(),
predicateResults: predicateResults,
},
}
return contract.Run(accessableState, env.Addresses().Caller, env.Addresses().Self, input, suppliedGas, env.ReadOnly())
Expand All @@ -140,7 +144,7 @@ func (r RulesExtra) PrecompileOverride(addr common.Address) (libevm.PrecompiledC

type accessableState struct {
env vm.PrecompileEnvironment
blockContext *BlockContext
blockContext *precompileBlockContext
}

func (a accessableState) GetStateDB() contract.StateDB {
Expand Down Expand Up @@ -170,28 +174,23 @@ func (a accessableState) Call(addr common.Address, input []byte, gas uint64, val
return a.env.Call(addr, input, gas, value)
}

type BlockContext struct {
number *big.Int
time uint64
predicateResultsBytes []byte
type precompileBlockContext struct {
number *big.Int
time uint64
predicateResults *predicate.Results
}

func NewBlockContext(number *big.Int, time uint64, predicateResultsBytes []byte) *BlockContext {
return &BlockContext{
number: number,
time: time,
predicateResultsBytes: predicateResultsBytes,
}
}

func (b *BlockContext) Number() *big.Int {
return b.number
func (p *precompileBlockContext) Number() *big.Int {
return p.number
}

func (b *BlockContext) Timestamp() uint64 {
return b.time
func (p *precompileBlockContext) Timestamp() uint64 {
return p.time
}

func (b *BlockContext) GetPredicateResultsBytes() []byte {
return b.predicateResultsBytes
func (p *precompileBlockContext) GetPredicateResults(txHash common.Hash, precompileAddress common.Address) []byte {
if p.predicateResults == nil {
return nil
}
return p.predicateResults.GetPredicateResults(txHash, precompileAddress)
}
25 changes: 25 additions & 0 deletions params/predicate_bytes.go
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the cyclic import that prevents us to have these in predicate pkg?

Copy link
Collaborator Author

@darioush darioush Oct 30, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

params in general is a really bad package for constants, and in our case it's even worse.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// (c) 2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package params

// GetPredicateResultBytes returns the predicate result bytes from extraData. If
// extraData is too short to include predicate results, it returns nil.
func GetPredicateResultBytes(extraData []byte) []byte {
// Prior to Durango, the VM enforces the extra data is smaller than or equal
// to this size.
if len(extraData) <= DynamicFeeExtraDataSize {
return nil
}
// After Durango, the extra data past the dynamic fee rollup window represents
// predicate results.
return extraData[DynamicFeeExtraDataSize:]
}

// SetPredicateResultBytes sets the predicate results in the extraData in the
// block header. This is used to set the predicate results in a block header
// without modifying the initial portion of the extra data (dynamic fee window
// rollup).
func SetPredicateResultBytes(extraData []byte, predicateResults []byte) []byte {
return append(extraData[:DynamicFeeExtraDataSize], predicateResults...)
}
25 changes: 25 additions & 0 deletions params/predicate_bytes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// (c) 2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package params

import (
"testing"

"github.com/ava-labs/avalanchego/utils"
"github.com/stretchr/testify/require"
)

func TestPredicateResultsBytes(t *testing.T) {
require := require.New(t)
dataTooShort := utils.RandomBytes(DynamicFeeExtraDataSize - 1)
resultBytes := GetPredicateResultBytes(dataTooShort)
require.Empty(resultBytes)

preDurangoData := utils.RandomBytes(DynamicFeeExtraDataSize)
resultBytes = GetPredicateResultBytes(preDurangoData)
require.Empty(resultBytes)
postDurangoData := utils.RandomBytes(DynamicFeeExtraDataSize + 2)
resultBytes = GetPredicateResultBytes(postDurangoData)
require.Equal(resultBytes, postDurangoData[DynamicFeeExtraDataSize:])
}
2 changes: 1 addition & 1 deletion plugin/evm/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func (b *Block) verifyPredicates(predicateContext *precompileconfig.PredicateCon
return fmt.Errorf("failed to marshal predicate results: %w", err)
}
extraData := b.ethBlock.Extra()
headerPredicateResultsBytes := predicate.GetPredicateResultBytes(extraData)
headerPredicateResultsBytes := params.GetPredicateResultBytes(extraData)
if len(headerPredicateResultsBytes) == 0 {
return fmt.Errorf("failed to find predicate results in extra data: %x", extraData)
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/evm/vm_warp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ func testReceiveWarpMessage(

// Require the block was built with a successful predicate result
ethBlock := block2.(*chain.BlockWrapper).Block.(*Block).ethBlock
headerPredicateResultsBytes := predicate.GetPredicateResultBytes(ethBlock.Extra())
headerPredicateResultsBytes := params.GetPredicateResultBytes(ethBlock.Extra())
require.NotEmpty(headerPredicateResultsBytes)
results, err := predicate.ParseResults(headerPredicateResultsBytes)
require.NoError(err)
Expand Down
6 changes: 3 additions & 3 deletions precompile/contract/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ type ConfigurationBlockContext interface {

type BlockContext interface {
ConfigurationBlockContext
// GetPredicateResults returns an byte array result of verifying the predicates
// of the given block.
GetPredicateResultsBytes() []byte
// GetPredicateResults returns a the result of verifying the predicates of the
// given transaction, precompile address pair as a byte array.
GetPredicateResults(txHash common.Hash, precompileAddress common.Address) []byte
}

type Configurator interface {
Expand Down
12 changes: 6 additions & 6 deletions precompile/contract/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading