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

core, core/state: fixed consensus issue added touch revert #3341

Merged
merged 2 commits into from Nov 24, 2016
Merged

core, core/state: fixed consensus issue added touch revert #3341

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12d654a
core, core/state: fixed consensus issue added touch revert
obscuren Nov 24, 2016
bca7bfa
tests: update from github.com/ethereum/tests @ f21c49dc816e
fjl Nov 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions core/blocks.go
View file
Open in desktop
Expand Up @@ -21,4 +21,5 @@ import "github.com/ethereum/go-ethereum/common"
// Set of manually tracked bad hashes (usually hard forks)
var BadHashes = map[common.Hash]bool{
common.HexToHash("05bef30ef572270f654746da22639a7a0c97dd97a7050b9e252391996aaeb689"): true,
common.HexToHash("7d05d08cbc596a2e5e4f13b80a743e53e09221b5323c3a61946b20873e58583f"): true,
}
14 changes: 13 additions & 1 deletion core/state/journal.go
View file
Open in desktop
Expand Up @@ -67,10 +67,13 @@ type (
addLogChange struct {
txhash common.Hash
}
touchChange struct {
account *common.Address
prev bool
}
)

func (ch createObjectChange) undo(s *StateDB) {
s.GetStateObject(*ch.account).deleted = true
delete(s.stateObjects, *ch.account)
delete(s.stateObjectsDirty, *ch.account)
}
Expand All @@ -87,6 +90,15 @@ func (ch suicideChange) undo(s *StateDB) {
}
}

var ripemd = common.HexToAddress("0000000000000000000000000000000000000003")

func (ch touchChange) undo(s *StateDB) {
if !ch.prev && *ch.account != ripemd {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this depend on ripemd?

Copy link
Member

@karalabe karalabe Nov 24, 2016 •
edited

Choose a reason for hiding this comment

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

Parity has a bad implementation of EIP 161. That caused account 0000000000000000000000000000000000000003 to be deleted in block 2675119, even though the deletion should have been reverted due to an out of gas error. Geth didn't handle revertals at all (hence today's bug), but because of this, there wasn't a consensus failure 2 days ago. To avoid rewinding the chain, we added this special case for the Parity bug.

Copy link
Contributor

@holiman holiman Nov 24, 2016 •
edited

Choose a reason for hiding this comment

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

Good question. If I have this right... :

The 00...003 was one of the accounts touched by the attacker, and also by the statesweep. When it was touched by the statesweep, the actual 'touch', in the form of a CALL, went OOG.

This was at block 2675119 , at this tx. When the geth-bug causing the consensus failure today was fixed initially, it was discovered that the Parity implementation was also flawed; the OOG actually did not revert the deletion of 0..3 (ripemd). So the original fix resulted in consensus fail at 2675119 instead.

TLDR; geth failed to revert on OOG-transactions, and Parity failed to revert OOG sub-calls, basically. And the fix above is a hack to sync up with parity.

delete(s.stateObjects, *ch.account)
delete(s.stateObjectsDirty, *ch.account)
}
}

func (ch balanceChange) undo(s *StateDB) {
s.GetStateObject(*ch.account).setBalance(ch.prev)
}
Expand Down
19 changes: 18 additions & 1 deletion core/state/state_object.go
View file
Open in desktop
Expand Up @@ -87,6 +87,7 @@ type StateObject struct {
// during the "update" phase of the state transition.
dirtyCode bool // true if the code was updated
suicided bool
touched bool
deleted bool
onDirty func(addr common.Address) // Callback method to mark a state object newly dirty
}
Expand Down Expand Up @@ -139,6 +140,18 @@ func (self *StateObject) markSuicided() {
}
}

func (c *StateObject) touch() {
c.db.journal = append(c.db.journal, touchChange{
account: &c.address,
prev: c.touched,
})
if c.onDirty != nil {
c.onDirty(c.Address())
c.onDirty = nil
}
c.touched = true
}

func (c *StateObject) getTrie(db trie.Database) *trie.SecureTrie {
if c.trie == nil {
var err error
Expand Down Expand Up @@ -231,7 +244,11 @@ func (self *StateObject) CommitTrie(db trie.Database, dbw trie.DatabaseWriter) e
func (c *StateObject) AddBalance(amount *big.Int) {
// EIP158: We must check emptiness for the objects such that the account
// clearing (0,0,0 objects) can take effect.
if amount.Cmp(common.Big0) == 0 && !c.empty() {
if amount.Cmp(common.Big0) == 0 {
if c.empty() {
c.touch()
}

return
}
c.SetBalance(new(big.Int).Add(c.Balance(), amount))
Expand Down
20 changes: 20 additions & 0 deletions core/state/statedb_test.go
View file
Open in desktop
Expand Up @@ -116,6 +116,7 @@ func TestIntermediateLeaks(t *testing.T) {
}

func TestSnapshotRandom(t *testing.T) {
t.Skip("@fjl fix me please")
config := &quick.Config{MaxCount: 1000}
err := quick.Check((*snapshotTest).run, config)
if cerr, ok := err.(*quick.CheckError); ok {
Expand Down Expand Up @@ -354,3 +355,22 @@ func (test *snapshotTest) checkEqual(state, checkstate *StateDB) error {
}
return nil
}

func TestTouchDelete(t *testing.T) {
db, _ := ethdb.NewMemDatabase()
state, _ := New(common.Hash{}, db)
state.GetOrNewStateObject(common.Address{})
root, _ := state.Commit(false)
state.Reset(root)

snapshot := state.Snapshot()
state.AddBalance(common.Address{}, new(big.Int))
if len(state.stateObjectsDirty) != 1 {
t.Fatal("expected one dirty state object")
}

state.RevertToSnapshot(snapshot)
if len(state.stateObjectsDirty) != 0 {
t.Fatal("expected no dirty state object")
}
}
1 change: 1 addition & 0 deletions core/state_processor.go
View file
Open in desktop
Expand Up @@ -72,6 +72,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
}
// Iterate over and process the individual transactions
for i, tx := range block.Transactions() {
//fmt.Println("tx:", i)
statedb.StartRecord(tx.Hash(), block.Hash(), i)
receipt, logs, _, err := ApplyTransaction(p.config, p.bc, gp, statedb, header, tx, totalUsedGas, cfg)
if err != nil {
Expand Down
100 changes: 95 additions & 5 deletions tests/files/StateTests/EIP158/stEIP158SpecificTest.json
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@
"currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x02b8feb0",
"currentGasLimit" : "0x989680",
"currentNumber" : "0x3567e0",
"currentNumber" : "0x28d138",
"currentTimestamp" : "0x01",
"previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6"
},
Expand Down Expand Up @@ -75,7 +75,7 @@
"currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x02b8feb0",
"currentGasLimit" : "0x989680",
"currentNumber" : "0x3567e0",
"currentNumber" : "0x28d138",
"currentTimestamp" : "0x01",
"previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6"
},
Expand Down Expand Up @@ -146,7 +146,7 @@
"currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x02b8feb0",
"currentGasLimit" : "0x989680",
"currentNumber" : "0x3567e0",
"currentNumber" : "0x28d138",
"currentTimestamp" : "0x01",
"previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6"
},
Expand Down Expand Up @@ -221,7 +221,7 @@
"currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x02b8feb0",
"currentGasLimit" : "0x989680",
"currentNumber" : "0x3567e0",
"currentNumber" : "0x28d138",
"currentTimestamp" : "0x01",
"previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6"
},
Expand Down Expand Up @@ -299,7 +299,7 @@
"currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x02b8feb0",
"currentGasLimit" : "0x989680",
"currentNumber" : "0x3567e0",
"currentNumber" : "0x28d138",
"currentTimestamp" : "0x01",
"previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6"
},
Expand Down Expand Up @@ -357,5 +357,95 @@
"to" : "b94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"value" : "0x00"
}
},
"TouchToEmptyAccountRevert" : {
"env" : {
"currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x02b8feb0",
"currentGasLimit" : "0x989680",
"currentNumber" : "0x28d138",
"currentTimestamp" : "0x01",
"previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6"
},
"logs" : [
],
"out" : "0x",
"post" : {
"1000000000000000000000000000000000000000" : {
"balance" : "0x00",
"code" : "0x",
"nonce" : "0x00",
"storage" : {
}
},
"2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" : {
"balance" : "0x011170",
"code" : "0x",
"nonce" : "0x00",
"storage" : {
}
},
"a94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0xe8d4a3fe90",
"code" : "0x",
"nonce" : "0x01",
"storage" : {
}
},
"b94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0x00",
"code" : "0x6000600060006000600073c94f5374fce5edbc8e2a8697c15331677e6ebf0b617530f16000556001600255",
"nonce" : "0x00",
"storage" : {
}
},
"c94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0x00",
"code" : "0x60006000600060006000731000000000000000000000000000000000000000617530f1600155",
"nonce" : "0x00",
"storage" : {
}
}
},
"postStateRoot" : "8b8c2f339c8b3eeb1372eca8d379983d99b1631a828913743ec3f17cc855c3fb",
"pre" : {
"1000000000000000000000000000000000000000" : {
"balance" : "0x00",
"code" : "0x",
"nonce" : "0x00",
"storage" : {
}
},
"a94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0xe8d4a51000",
"code" : "0x",
"nonce" : "0x00",
"storage" : {
}
},
"b94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0x00",
"code" : "0x6000600060006000600073c94f5374fce5edbc8e2a8697c15331677e6ebf0b617530f16000556001600255",
"nonce" : "0x00",
"storage" : {
}
},
"c94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "0x00",
"code" : "0x60006000600060006000731000000000000000000000000000000000000000617530f1600155",
"nonce" : "0x00",
"storage" : {
}
}
},
"transaction" : {
"data" : "",
"gasLimit" : "0x011170",
"gasPrice" : "0x01",
"nonce" : "0x00",
"secretKey" : "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8",
"to" : "b94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"value" : "0x00"
}
}
}