Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions .changeset/old-mirrors-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@eth-optimism/integration-tests": patch
"@eth-optimism/l2geth": patch
---

Correctly set the OVM context based on the L1 values during `eth_call`. This will also set it during `eth_estimateGas`. Add tests for this in the integration tests
29 changes: 27 additions & 2 deletions integration-tests/test/ovmcontext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,32 @@ describe('OVM Context: Layer 2 EVM Context', () => {
}
})

it('should set correct OVM Context for `eth_call`', async () => {
const tip = await L2Provider.getBlockWithTransactions('latest')
const start = Math.max(0, tip.number - 5)

for (let i = start; i < tip.number; i++) {
const block = await L2Provider.getBlockWithTransactions(i)
const [, returnData] = await OVMMulticall.callStatic.aggregate([
[
OVMMulticall.address,
OVMMulticall.interface.encodeFunctionData('getCurrentBlockTimestamp'),
],
[
OVMMulticall.address,
OVMMulticall.interface.encodeFunctionData('getCurrentBlockNumber'),
],
], {blockTag: i})

const timestamp = BigNumber.from(returnData[0])
const blockNumber = BigNumber.from(returnData[1])
const tx = block.transactions[0] as any

expect(tx.l1BlockNumber).to.deep.equal(blockNumber.toNumber())
expect(block.timestamp).to.deep.equal(timestamp.toNumber())
}
})

/**
* `rollup_getInfo` is a new RPC endpoint that is used to return the OVM
* context. The data returned should match what is actually being used as the
Expand Down Expand Up @@ -126,8 +152,7 @@ describe('OVM Context: Layer 2 EVM Context', () => {
const timestamp = BigNumber.from(returnData[0])
const blockNumber = BigNumber.from(returnData[1])

// TODO: this is a bug and needs to be fixed
//expect(info.ethContext.blockNumber).to.deep.equal(blockNumber.toNumber())
expect(info.ethContext.blockNumber).to.deep.equal(blockNumber.toNumber())
expect(info.ethContext.timestamp).to.deep.equal(timestamp.toNumber())
})
})
2 changes: 1 addition & 1 deletion l2geth/core/state_transition_ovm.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func EncodeSimulatedMessage(msg Message, timestamp, blockNumber *big.Int, execut

tx := ovmTransaction{
timestamp,
blockNumber, // TODO (what's the correct block number?)
blockNumber,
uint8(msg.QueueOrigin().Uint64()),
*msg.L1MessageSender(),
*to,
Expand Down
21 changes: 18 additions & 3 deletions l2geth/internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -923,16 +923,29 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo
data = []byte(*args.Data)
}

blockNumber := header.Number
timestamp := new(big.Int).SetUint64(header.Time)

// Create new call message
var msg core.Message
msg = types.NewMessage(addr, args.To, 0, value, gas, gasPrice, data, false, &addr, nil, types.QueueOriginSequencer, 0)
if vm.UsingOVM {
cfg := b.ChainConfig()
executionManager := cfg.StateDump.Accounts["OVM_ExecutionManager"]
stateManager := cfg.StateDump.Accounts["OVM_StateManager"]
var err error
blockNumber := header.Number
timestamp := new(big.Int).SetUint64(header.Time)
block, err := b.BlockByNumber(ctx, rpc.BlockNumber(header.Number.Uint64()))
if err != nil {
return nil, 0, false, err
}
txs := block.Transactions()
if header.Number.Uint64() != 0 {
if len(txs) != 1 {
return nil, 0, false, fmt.Errorf("block %d has more than 1 transaction", header.Number.Uint64())
}
tx := txs[0]
blockNumber = tx.L1BlockNumber()
timestamp = new(big.Int).SetUint64(tx.L1Timestamp())
}
msg, err = core.EncodeSimulatedMessage(msg, timestamp, blockNumber, executionManager, stateManager)
if err != nil {
return nil, 0, false, err
Expand Down Expand Up @@ -968,6 +981,8 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo
gp := new(core.GasPool).AddGas(math.MaxUint64)
if vm.UsingOVM {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need a less error prone way of getting an EVM with the correct context set, otherwise we need to make sure to mutate the evm context externally in each codepath that it is used. Thinking about the correct place to slot in will be left to the berlin geth migration

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you open an issue describing the desired solution you have in mind?

evm.Context.EthCallSender = &addr
evm.Context.BlockNumber = blockNumber
evm.Context.Time = timestamp
}
res, gas, failed, err := core.ApplyMessage(evm, msg, gp)
if err := vmError(); err != nil {
Expand Down