-
Notifications
You must be signed in to change notification settings - Fork 13
Replay for Byzantium data #436
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
base: jsign-replay-kaustinen6
Are you sure you want to change the base?
Changes from all commits
3747a9c
a190055
28865fe
b9e361c
cd92db3
3ee2710
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -185,7 +185,7 @@ func InsertBlockHashHistoryAtEip2935Fork(statedb *state.StateDB, prevNumber uint | |
statedb.Witness().TouchFullAccount(params.HistoryStorageAddress[:], true) | ||
|
||
ancestor := chain.GetHeader(prevHash, prevNumber) | ||
for i := prevNumber; i > 0 && i >= prevNumber-params.Eip2935BlockHashHistorySize; i-- { | ||
for i := prevNumber; i > 0 && i > prevNumber-params.Eip2935BlockHashHistorySize; i-- { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be removed when #438 is merged and the base branch change bubbles up when we rebase branches. For now leaving the fix since it's needed. |
||
ProcessParentBlockHash(statedb, i, ancestor.Hash()) | ||
ancestor = chain.GetHeader(ancestor.ParentHash, ancestor.Number.Uint64()-1) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -377,6 +377,8 @@ func gasExpEIP158(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memor | |
return gas, nil | ||
} | ||
|
||
var replayMode = true | ||
|
||
func gasCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) { | ||
var ( | ||
gas uint64 | ||
|
@@ -391,7 +393,7 @@ func gasCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize | |
} else if !evm.StateDB.Exist(address) { | ||
gas += params.CallNewAccountGas | ||
} | ||
if transfersValue && !evm.chainRules.IsEIP4762 { | ||
if transfersValue && (!evm.chainRules.IsEIP4762 || replayMode) { | ||
Comment on lines
-394
to
+396
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other case that we should disable the effects. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but isn't just deactivating eip 4762 enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note to self: no it isn't, because if you do then eip2929 is still active and we don't want to handle that case. I guess eip2929 becomes the new final boss. |
||
gas += params.CallValueTransferGas | ||
} | ||
memoryGas, err := memoryGasCost(mem, memorySize) | ||
|
@@ -431,7 +433,7 @@ func gasCallCode(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memory | |
gas uint64 | ||
overflow bool | ||
) | ||
if stack.Back(2).Sign() != 0 && !evm.chainRules.IsEIP4762 { | ||
if stack.Back(2).Sign() != 0 && (!evm.chainRules.IsEIP4762 || replayMode) { | ||
jsign marked this conversation as resolved.
Show resolved
Hide resolved
|
||
gas += params.CallValueTransferGas | ||
} | ||
if gas, overflow = math.SafeAdd(gas, memoryGas); overflow { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a required change to avoid an error when processing the second block.
Before merging we have to figure out a coherent way to do this for both replay and non-replay setups to work correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed. but if it's the only change that we have to keep in a separate branch, that's ok too.