EIP158 & 160 Hardfork #3179
EIP158 & 160 Hardfork #3179
Conversation
|
@obscuren, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fjl, @karalabe and @zsfelfoldi to be potential reviewers. |
obscuren
force-pushed
the
eip-158
branch
17 times, most recently
from
00b71cf
to
97b4109
Compare
obscuren
force-pushed
the
eip-158
branch
6 times, most recently
from
eda9c0f
to
2e6397f
Compare
|
|
||
| // Nothing should be returned when an error is thrown. | ||
| return nil, addr, err | ||
| } else if env.ChainConfig().IsEIP170(env.BlockNumber()) && len(ret) > params.MaxCodeSize { |
There was a problem hiding this comment.
A few lines up, back at L:157, there's if err == nil, where the clause which does setCode is performed. I suggest to check MaxCodeSize up there instead of down here, and then also skip the SetCode stuff.
obscuren
force-pushed
the
eip-158
branch
5 times, most recently
from
7f8828b
to
e6c838f
Compare
| EIP155Block *big.Int `json:"EIP155Block"` // EIP155 HF block | ||
| EIP158Block *big.Int `json:"EIP158Block"` // EIP158 HF block | ||
| EIP155Block *big.Int `json:"eip155Block"` // EIP155 HF block | ||
| EIP158Block *big.Int `json:"eiP158Block"` // EIP158 HF block |
| if tx.Protected() { | ||
| signer = types.NewEIP155Signer(tx.ChainId()) | ||
| } | ||
| if _, r, _ := types.SignatureValues(signer, tx); r == nil { |
There was a problem hiding this comment.
You can use RawSignatureValues here (and below). The point of this check is to see whether the fields were set in JSON.
| signer = types.NewEIP155Signer(tx.ChainId()) | ||
| } | ||
| from, _ := types.Sender(signer, tx) | ||
| v, r, s := types.SignatureValues(signer, tx) |
There was a problem hiding this comment.
I think this should be RawSignatureValues. The v, r, s JSON fields are new in 1.5 and we can define the semantics for them. Raw values are preferable here because the client should be able to reconstruct the whole transaction.
e.g. if you do
client, _ := ethclient.Dial("http://127.0.0.1:8085")
tx, _ := client.TransactionByHash(ctx, common.HexToHash("...."))
from, _ := types.Sender(types.NewEIP155Signer(signer), tx))The from value should be the same as it was on the server. Normalising the values breaks this.
obscuren
force-pushed
the
eip-158
branch
2 times, most recently
from
7449ade
to
aa3017c
Compare
| @@ -40,10 +40,11 @@ type ChainConfig struct { | |||
|
|
|||
| EIP155Block *big.Int `json:"eip155Block"` // EIP155 HF block | |||
| EIP158Block *big.Int `json:"eiP158Block"` // EIP158 HF block | |||
| EIP170Block *big.Int `json:"eip158Block"` // EIP170 HF block | |||
There was a problem hiding this comment.
s/eip158Block/eip170Block no?
obscuren
force-pushed
the
eip-158
branch
3 times, most recently
from
f108749
to
75bf510
Compare
This commit implements EIP158 part 1, 2, 3 & 4 1. If an account is empty it's no longer written to the trie. An empty account is defined as (balance=0, nonce=0, storage=0, code=0). 2. Delete an empty account if it's touched 3. An empty account is redefined as either non-existent or empty. 4. Zero value calls and zero value suicides no longer consume the 25k reation costs. params: moved core/config to params Signed-off-by: Jeffrey Wilcke <jeffrey@ethereum.org>
|
@obscuren please fix the go vet issue |
This PR implements the EIP158 proposal and EIP160.
It also changes the location where the chain configuration is kept to the
paramspackage. This is done so that any time a HF proposal is added or if the chain config needs modification, only a single location needs to be changed. This also means that thevm.Environmentno longer has aRuleSetbut aChainConfigdirectly. All locations in the tests wherecore.GenerateChaintook anilparameter for theChainConfigare now passingparams.TestChainConfig.ChainConfigis no longer optional, it is an requirement. Passingnilanywhere (as done in mostGenerateChaincalls) will result in a hard panic.Note for reviewers: please make sure that a
params.TestChainConfigis suitable for your tests. It has the following parameters:homestead = 0, dao = 0, daoEnabled = true, EIP150 = true, EIP158 = true.TODO list
core.ChainConfigand moved toparamspackage instead