-
Notifications
You must be signed in to change notification settings - Fork 63
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
support for golang tracers + add golang callTracer #558
Changes from 12 commits
96c6b9c
95eb5d0
a2dba52
89afd2e
e88bdbd
fec009d
afa19f8
8d176f7
e1b7ba6
6bdee24
0c7c95d
de2be8f
89fb632
056e16d
a3d48d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ package core | |
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"math" | ||
"math/big" | ||
|
||
|
@@ -100,13 +101,13 @@ func IntrinsicGas(data []byte, accessList types.AccessList, isContractCreation, | |
} | ||
// Make sure we don't exceed uint64 for all data combinations | ||
if (math.MaxUint64-gas)/params.TxDataNonZeroGas < nz { | ||
return 0, vm.ErrOutOfGas | ||
return 0, ErrGasUintOverflow | ||
} | ||
gas += nz * params.TxDataNonZeroGas | ||
|
||
z := uint64(len(data)) - nz | ||
if (math.MaxUint64-gas)/params.TxDataZeroGas < z { | ||
return 0, vm.ErrOutOfGas | ||
return 0, ErrGasUintOverflow | ||
} | ||
gas += z * params.TxDataZeroGas | ||
} | ||
|
@@ -169,15 +170,6 @@ func (st *StateTransition) to() vm.AccountRef { | |
return reference | ||
} | ||
|
||
func (st *StateTransition) useGas(amount uint64) error { | ||
if st.gas < amount { | ||
return vm.ErrOutOfGas | ||
} | ||
st.gas -= amount | ||
|
||
return nil | ||
} | ||
|
||
func (st *StateTransition) buyGas() error { | ||
var ( | ||
state = st.state | ||
|
@@ -238,9 +230,10 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG | |
if err != nil { | ||
return nil, 0, false, err, nil | ||
} | ||
if err = st.useGas(gas); err != nil { | ||
return nil, 0, false, err, nil | ||
if st.gas < gas { | ||
return nil, 0, false, fmt.Errorf("%w: have %d, want %d", ErrIntrinsicGas, st.gas, gas), nil | ||
} | ||
st.gas -= gas | ||
|
||
if rules := st.evm.ChainConfig().Rules(st.evm.Context.BlockNumber); rules.IsEIP1559 { | ||
st.state.PrepareAccessList(msg.From(), msg.To(), vm.ActivePrecompiles(rules), msg.AccessList()) | ||
|
@@ -273,7 +266,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG | |
// sufficient balance to make the transfer happen. The first | ||
// balance transfer may never fail. | ||
if vmerr == vm.ErrInsufficientBalance { | ||
return nil, 0, false, vmerr, nil | ||
return nil, 0, false, nil, vmerr | ||
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 you explain this change? It is a sensitive part of the code. 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. I am convinced this will introduce a bug and should be resolved. 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. I agree. I checked history geth commit: ethereum@b9df7ec |
||
} | ||
} | ||
st.refundGas() | ||
|
@@ -286,7 +279,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG | |
st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(new(big.Int).SetUint64(st.gasUsed()), st.gasPrice)) | ||
} | ||
|
||
return ret, st.gasUsed(), vmerr != nil, err, vmerr | ||
return ret, st.gasUsed(), vmerr != nil, nil, vmerr | ||
} | ||
|
||
func (st *StateTransition) refundGas() { | ||
|
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.
Why did you have to change these parts?
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 change is for preparation of EIP2028. The reason we need EIP2028 is as follows:
If EIP2028 is not enabled, the test
revert_reason.json
will not pass unless we change the test itself. Check this commit that change the test json itself: 0a496c3So actually this code change does not necessary belong to this PR. Do you want me to undo it?
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.
The consensus will be changed if we enable eip-2028. I can include it in the PR of EIP-1559 if you need it.
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.
Yes please let's remove that from this PR. It would be a separate discussion if EIP-2028 is useful for XDC.
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.
I removed it and force pushed commits.
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.
No hurry. At the time of eip2028, we need to remember to undo this commit: e1b7ba6