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

Jump to bottom

Verkle EXTCODECOPY implementation #55

Merged
merged 4 commits into from
Jan 13, 2022
Merged

Verkle EXTCODECOPY implementation #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c3b263
core/vm: verkle extcodecopy naive way (do jumpdest analysis on target…
jwasinger Jan 11, 2022
0210a87
no double-charge
jwasinger Jan 11, 2022
c10eed3
address edge-case in touchEachChunksAndChargeGas
jwasinger Jan 12, 2022
36265f4
simplify line
gballet Jan 13, 2022
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/vm/analysis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ func (bits bitvec) set16(pos uint64) {
bits[pos/8+2] = ^a
}

// codeSegment checks if the position is in a code segment.
func (bits *bitvec) codeSegment(pos uint64) bool {
// IsCode checks if the position is in a code segment.
func (bits *bitvec) IsCode(pos uint64) bool {
return ((*bits)[pos/8] & (0x80 >> (pos % 8))) == 0
}

Expand Down
13 changes: 10 additions & 3 deletions core/vm/contract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ type ContractRef interface {
Address() common.Address
}

// AnalyzedContract is an interface for a piece of contract
// code that has undegone jumpdest analysis, and whose bytecode
// can be queried to determine if it is "contract code"
type AnalyzedContract interface {
IsCode(dest uint64) bool
}

// AccountRef implements ContractRef.
//
// Account references are used during EVM initialisation and
Expand Down Expand Up @@ -101,7 +108,7 @@ func (c *Contract) validJumpdest(dest *uint256.Int) bool {
func (c *Contract) IsCode(udest uint64) bool {
// Do we already have an analysis laying around?
if c.analysis != nil {
return c.analysis.codeSegment(udest)
return c.analysis.IsCode(udest)
}
// Do we have a contract hash already?
// If we do have a hash, that means it's a 'regular' contract. For regular
Expand All @@ -117,7 +124,7 @@ func (c *Contract) IsCode(udest uint64) bool {
}
// Also stash it in current contract for faster access
c.analysis = analysis
return analysis.codeSegment(udest)
return analysis.IsCode(udest)
}
// We don't have the code hash, most likely a piece of initcode not already
// in state trie. In that case, we do an analysis, and save it locally, so
Expand All @@ -126,7 +133,7 @@ func (c *Contract) IsCode(udest uint64) bool {
if c.analysis == nil {
c.analysis = codeBitmap(c.Code)
}
return c.analysis.codeSegment(udest)
return c.analysis.IsCode(udest)
}

// AsDelegate sets the contract to be a delegate call and returns the current
Expand Down
5 changes: 3 additions & 2 deletions core/vm/gas_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func gasCodeCopy(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memory
uint64Length = 0xffffffffffffffff
}
_, offset, nonPaddedSize := getDataAndAdjustedBounds(contract.Code, uint64CodeOffset, uint64Length)
statelessGas = touchEachChunksAndChargeGas(offset, nonPaddedSize, contract.Address().Bytes()[:], nil, evm.Accesses)
statelessGas = touchEachChunksAndChargeGas(offset, nonPaddedSize, contract.Address().Bytes()[:], nil, nil, evm.Accesses)
}
usedGas, err := gasCodeCopyStateful(evm, contract, stack, mem, memorySize)
return usedGas + statelessGas, err
Expand All @@ -133,6 +133,7 @@ func gasExtCodeCopy(evm *EVM, contract *Contract, stack *Stack, mem *Memory, mem
var (
codeOffset = stack.Back(2)
length = stack.Back(3)
targetAddr = stack.Back(0).Bytes20()
)
uint64CodeOffset, overflow := codeOffset.Uint64WithOverflow()
if overflow {
Expand All @@ -148,7 +149,7 @@ func gasExtCodeCopy(evm *EVM, contract *Contract, stack *Stack, mem *Memory, mem
// behavior from CODECOPY which only charges witness access costs for the part of the range
// which overlaps in the account code. TODO: clarify this is desired behavior and amend the
// spec.
statelessGas = touchEachChunksAndChargeGas(uint64CodeOffset, uint64Length, nil, nil, evm.Accesses)
statelessGas = touchEachChunksAndChargeGas(uint64CodeOffset, uint64Length, targetAddr[:], nil, nil, evm.Accesses)
}
usedGas, err := gasExtCodeCopyStateful(evm, contract, stack, mem, memorySize)
return usedGas + statelessGas, err
Expand Down
27 changes: 13 additions & 14 deletions core/vm/instructions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package vm
import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/params"
trieUtils "github.com/ethereum/go-ethereum/trie/utils"
"github.com/holiman/uint256"
Expand Down Expand Up @@ -373,21 +372,21 @@ func opCodeCopy(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([

paddedCodeCopy, copyOffset, nonPaddedCopyLength := getDataAndAdjustedBounds(scope.Contract.Code, uint64CodeOffset, length.Uint64())
if interpreter.evm.Accesses != nil {
touchEachChunksAndChargeGas(copyOffset, nonPaddedCopyLength, scope.Contract.Address().Bytes()[:], scope.Contract, interpreter.evm.Accesses)
touchEachChunksAndChargeGas(copyOffset, nonPaddedCopyLength, scope.Contract.Address().Bytes()[:], scope.Contract.Code, scope.Contract, interpreter.evm.Accesses)
}
scope.Memory.Set(memOffset.Uint64(), uint64(len(paddedCodeCopy)), paddedCodeCopy)
return nil, nil
}

// touchEachChunksAndChargeGas is a helper function to touch every chunk in a code range and charge witness gas costs
func touchEachChunksAndChargeGas(offset, size uint64, address []byte, contract *Contract, accesses *types.AccessWitness) uint64 {
func touchEachChunksAndChargeGas(offset, size uint64, address []byte, code []byte, contract AnalyzedContract, accesses *types.AccessWitness) uint64 {
Copy link
Owner

Choose a reason for hiding this comment

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

Why add an extra parameter here? You could add a Code() method to AnalyzedContract.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because then I will need to add a Code() method to Contract for it to be AnalyzedContract which is redundant b/c Contract.Code is public. or create a separate object that wraps Contract and implements Code(), pass that object every time I call touchEachChunksAndChargeGas.

The former option isn't too bad but it's also kind of awkward.

// note that in the case where the copied code is outside the range of the
// contract code but touches the last leaf with contract code in it,
// we don't include the last leaf of code in the AccessWitness. The
// reason that we do not need the last leaf is the account's code size
// is already in the AccessWitness so a stateless verifier can see that
// the code from the last leaf is not needed.
if contract != nil && (size == 0 || offset > uint64(len(contract.Code))) {
if contract != nil && (size == 0 || offset > uint64(len(code))) {
return 0
}
var (
Expand All @@ -397,17 +396,13 @@ func touchEachChunksAndChargeGas(offset, size uint64, address []byte, contract *
startOffset uint64
endOffset uint64
numLeaves uint64
code []byte
index [32]byte
)
if contract != nil {
code = contract.Code[:]
}
// startLeafOffset, endLeafOffset is the evm code offset of the first byte in the first leaf touched
// and the evm code offset of the last byte in the last leaf touched
startOffset = offset - (offset % 31)
if contract != nil && startOffset+size > uint64(len(contract.Code)) {
endOffset = uint64(len(contract.Code))
if contract != nil && startOffset+size > uint64(len(code)) {
endOffset = uint64(len(code))
} else {
endOffset = startOffset + size
}
Expand All @@ -431,11 +426,11 @@ func touchEachChunksAndChargeGas(offset, size uint64, address []byte, contract *
index[31] = subIndex

var value []byte
if contract != nil {
if len(code) > 0 {
// the offset into the leaf that the first PUSH occurs
var firstPushOffset uint64 = 0
// Look for the first code byte (i.e. no pushdata)
for ; firstPushOffset < 31 && firstPushOffset+uint64(i)*31 < uint64(len(contract.Code)) && !contract.IsCode(uint64(i)*31+firstPushOffset); firstPushOffset++ {
for ; firstPushOffset < 31 && firstPushOffset+uint64(i)*31 < uint64(len(code)) && !contract.IsCode(uint64(i)*31+firstPushOffset); firstPushOffset++ {
}
curEnd := (uint64(i) + 1) * 31
if curEnd > endOffset {
Expand Down Expand Up @@ -472,7 +467,11 @@ func opExtCodeCopy(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext)
}
addr := common.Address(a.Bytes20())
if interpreter.evm.Accesses != nil {
log.Warn("setting witness values for extcodecopy is not currently implemented")
code := interpreter.evm.StateDB.GetCode(addr)
paddedCodeCopy, copyOffset, nonPaddedCopyLength := getDataAndAdjustedBounds(code, uint64CodeOffset, length.Uint64())
cb := codeBitmap(code)
touchEachChunksAndChargeGas(copyOffset, nonPaddedCopyLength, addr[:], code, &cb, interpreter.evm.Accesses)
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think you actually need to do the analysis here, nor to charge the gas at this point, or to know what in the code is pushdata and what is code.

As long as you know the start offset, you can divide it into 31-byte chunks. I might be missing something, so we can have a call about it.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah ok, now I get it: one needs to know what the value of the pushdata offset it. Hmmm....

scope.Memory.Set(memOffset.Uint64(), length.Uint64(), paddedCodeCopy)
} else {
codeCopy := getData(interpreter.evm.StateDB.GetCode(addr), uint64CodeOffset, length.Uint64())
scope.Memory.Set(memOffset.Uint64(), length.Uint64(), codeCopy)
Expand Down Expand Up @@ -977,7 +976,7 @@ func makePush(size uint64, pushByteSize int) executionFunc {
}

if interpreter.evm.Accesses != nil {
statelessGas := touchEachChunksAndChargeGas(uint64(startMin), uint64(pushByteSize), scope.Contract.Address().Bytes()[:], scope.Contract, interpreter.evm.Accesses)
statelessGas := touchEachChunksAndChargeGas(uint64(startMin), uint64(pushByteSize), scope.Contract.Address().Bytes()[:], scope.Contract.Code, scope.Contract, interpreter.evm.Accesses)
scope.Contract.UseGas(statelessGas)
}

Expand Down
2 changes: 1 addition & 1 deletion core/vm/interpreter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
if in.evm.TxContext.Accesses != nil {
// if the PC ends up in a new "page" of verkleized code, charge the
// associated witness costs.
contract.Gas -= touchEachChunksAndChargeGas(pc, 1, contract.Address().Bytes()[:], contract, in.evm.TxContext.Accesses)
contract.Gas -= touchEachChunksAndChargeGas(pc, 1, contract.Address().Bytes()[:], contract.Code, contract, in.evm.TxContext.Accesses)
}

// TODO how can we tell if we are in stateless mode here and need to get the op from the witness
Expand Down