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.

Sign up for GitHub

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

pass extra bytes directly to precompiles #675

Closed
wants to merge 1 commit into from
Closed

pass extra bytes directly to precompiles #675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 3 deletions eth/tracers/api.go
View file
Open in desktop
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,8 +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)
blockContext := params.NewBlockContext(vmctx.BlockNumber, vmctx.Time, vmctx.Header.Extra)
err = core.ApplyUpgrades(api.backend.ChainConfig(), &originalTime, blockContext, statedb)
if err != nil {
return nil, err
Expand Down
28 changes: 12 additions & 16 deletions params/hooks_libevm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,12 @@ 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:]
}
accessableState := accessableState{
env: env,
blockContext: &BlockContext{
number: env.BlockNumber(),
time: env.BlockTime(),
predicateResultsBytes: predicateResultsBytes,
number: env.BlockNumber(),
time: env.BlockTime(),
extra: header.Extra,
},
}
return contract.Run(accessableState, env.Addresses().Caller, env.Addresses().Self, input, suppliedGas, env.ReadOnly())
Expand Down Expand Up @@ -220,16 +216,16 @@ func (a accessableState) NativeAssetCall(caller common.Address, input []byte, su
}

type BlockContext struct {
number *big.Int
time uint64
predicateResultsBytes []byte
number *big.Int
time uint64
extra []byte
}

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

Expand All @@ -241,6 +237,6 @@ func (b *BlockContext) Timestamp() uint64 {
return b.time
}

func (b *BlockContext) GetPredicateResultsBytes() []byte {
return b.predicateResultsBytes
func (b *BlockContext) Extra() []byte {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this PR conflicts with my comment here

If we can extend the BlockContext here, why we avoid using full and parsed types here?

return b.extra
}
4 changes: 1 addition & 3 deletions precompile/contract/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ type ConfigurationBlockContext interface {

type BlockContext interface {
ConfigurationBlockContext
// GetPredicateResults returns an byte array result of verifying the predicates
// of the given block.
GetPredicateResultsBytes() []byte
Extra() []byte // Exta returns the Extra field of the block header
Copy link
Contributor

Choose a reason for hiding this comment

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

How about HeaderExtra() []byte and remove the comment? That way it's also implicitly documented at the call site.

}

type Configurator interface {
Expand Down
12 changes: 6 additions & 6 deletions precompile/contract/mocks.go
View file
Open in desktop

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

50 changes: 26 additions & 24 deletions precompile/contracts/warp/contract_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/ava-labs/avalanchego/vms/platformvm/warp"
"github.com/ava-labs/avalanchego/vms/platformvm/warp/payload"
"github.com/ava-labs/coreth/core/extstate"
"github.com/ava-labs/coreth/params"
"github.com/ava-labs/coreth/precompile/contract"
"github.com/ava-labs/coreth/precompile/testutils"
"github.com/ava-labs/coreth/predicate"
Expand Down Expand Up @@ -171,7 +172,7 @@ func TestSendWarpMessage(t *testing.T) {
testutils.RunPrecompileTests(t, Module, extstate.NewTestStateDB, tests)
}

func toResultsBytes(txResults []byte) []byte {
func toExtra(txResults []byte) []byte {
results, err := predicate.NewResultsFromMap(
map[common.Hash]predicate.TxResults{
common.Hash{}: {ContractAddress: txResults},
Expand All @@ -180,7 +181,8 @@ func toResultsBytes(txResults []byte) []byte {
if err != nil {
panic(err)
}
return results
padding := make([]byte, params.DynamicFeeExtraDataSize)
return predicate.SetPredicateResultBytes(padding, results)
}

func TestGetVerifiedWarpMessage(t *testing.T) {
Expand Down Expand Up @@ -212,7 +214,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)),
ReadOnly: false,
Expand Down Expand Up @@ -242,7 +244,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: false,
Expand All @@ -265,7 +267,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{{}, warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(set.NewBits(0).Bytes()))
mbc.EXPECT().Extra().Return(toExtra(set.NewBits(0).Bytes()))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)),
ReadOnly: false,
Expand Down Expand Up @@ -295,7 +297,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{{}, warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(set.NewBits(0, 1).Bytes()))
mbc.EXPECT().Extra().Return(toExtra(set.NewBits(0, 1).Bytes()))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: false,
Expand All @@ -311,7 +313,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
Caller: callerAddr,
InputFn: func(t testing.TB) []byte { return getVerifiedWarpMsg },
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: false,
Expand All @@ -330,7 +332,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)),
ReadOnly: true,
Expand All @@ -353,7 +355,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
Caller: callerAddr,
InputFn: func(t testing.TB) []byte { return getVerifiedWarpMsg },
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: true,
Expand Down Expand Up @@ -382,7 +384,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)) - 1,
ReadOnly: false,
Expand All @@ -395,7 +397,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessage.Bytes()})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessage.Bytes())),
ReadOnly: false,
Expand All @@ -408,7 +410,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{predicate.PackPredicate([]byte{1, 2, 3})})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(32),
ReadOnly: false,
Expand All @@ -426,7 +428,7 @@ func TestGetVerifiedWarpMessage(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{predicate.PackPredicate(warpMessage.Bytes())})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(160),
ReadOnly: false,
Expand Down Expand Up @@ -493,7 +495,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)),
ReadOnly: false,
Expand Down Expand Up @@ -522,7 +524,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: false,
Expand All @@ -545,7 +547,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{{}, warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(set.NewBits(0).Bytes()))
mbc.EXPECT().Extra().Return(toExtra(set.NewBits(0).Bytes()))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)),
ReadOnly: false,
Expand Down Expand Up @@ -574,7 +576,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{{}, warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(set.NewBits(0, 1).Bytes()))
mbc.EXPECT().Extra().Return(toExtra(set.NewBits(0, 1).Bytes()))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: false,
Expand All @@ -590,7 +592,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
Caller: callerAddr,
InputFn: func(t testing.TB) []byte { return getVerifiedWarpBlockHash },
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: false,
Expand All @@ -609,7 +611,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)),
ReadOnly: true,
Expand All @@ -631,7 +633,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
Caller: callerAddr,
InputFn: func(t testing.TB) []byte { return getVerifiedWarpBlockHash },
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost,
ReadOnly: true,
Expand Down Expand Up @@ -660,7 +662,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessagePredicateBytes})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessagePredicateBytes)) - 1,
ReadOnly: false,
Expand All @@ -673,7 +675,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{warpMessage.Bytes()})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(len(warpMessage.Bytes())),
ReadOnly: false,
Expand All @@ -686,7 +688,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{predicate.PackPredicate([]byte{1, 2, 3})})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(32),
ReadOnly: false,
Expand All @@ -704,7 +706,7 @@ func TestGetVerifiedWarpBlockHash(t *testing.T) {
state.SetPredicateStorageSlots(ContractAddress, [][]byte{predicate.PackPredicate(warpMessage.Bytes())})
},
SetupBlockContext: func(mbc *contract.MockBlockContext) {
mbc.EXPECT().GetPredicateResultsBytes().Return(toResultsBytes(noFailures))
mbc.EXPECT().Extra().Return(toExtra(noFailures))
},
SuppliedGas: GetVerifiedWarpMessageBaseCost + GasCostPerWarpMessageBytes*uint64(160),
ReadOnly: false,
Expand Down
3 changes: 2 additions & 1 deletion precompile/contracts/warp/contract_warp_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ func handleWarpMessage(accessibleState contract.AccessibleState, input []byte, s
warpIndex := int(warpIndexInput) // This conversion is safe even if int is 32 bits because we checked above.
state := accessibleState.GetStateDB()
predicateBytes, exists := state.GetPredicateStorageSlots(ContractAddress, warpIndex)
predicateResultsBytes := accessibleState.GetBlockContext().GetPredicateResultsBytes()
extra := accessibleState.GetBlockContext().Extra()
predicateResultsBytes := predicate.GetPredicateResultBytes(extra)
Comment on lines +64 to +65
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IMO, the logic in this file from line 63-72, should be moved somewhere else.

ie, this contract is called in the context of some block and some tx, so it should not have to specify the hash of the tx currently being executed to the statedb to query it for the predicates, then query the results from the block header.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I'm happy to review that as part of this PR, or as a later one if you'd like. I'm approving in case you choose the latter so please ping me if you want a re-review.

Copy link
Collaborator

@ceyonur ceyonur Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IMHO:

  • warp contract should not need to deal with parsing predicate results (we already parse them for verification). I'm looking at the upstream and seems they also don't include header in the block context. If we can be flexible in BlockContext (we should be), we don't actually need to abstract everything. I do think BlockContext should be the place where we can have custom stuff derived from block/header to present for the execution.
  • warp contract should not know about extra at all
  • Predicaters does not need to be just for warp messaging, any other predicater should not re-implement all this parse-from-extra logic.

These are my observations after looking at just diffs, block context and warp handler. I haven't considered libevm or any other cyclic dependency. LMK if there is another concern that outweighs these considerations (especially parsing).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This approach #679 tries to follow the comment above, let me know your thoughts.

predicateResults, err := predicate.ParseResults(predicateResultsBytes)
if err != nil {
// Note this should never occur as the results are parsed in the block header.
Expand Down
Loading