core/state: accumulate writes and only update tries when must

[karalabe]

Byzantium removed the intermediate hashes to avoid having to reconstruct the state tries after each transaction. We indeed avoid hashing all the intermediate tries now (which saves CPU time), but we still reconstruct the tries in memory.

There's no reason for this, since subsequent transactions might modify the same slots. Pushing the updates into the storage tries after each transaction also makes it impossible to make these concurrent, since any single transaction will not modify enough accounts to make it worthwhile.

This PR lays the groundwork for the concurrency PR later on, where we cache all the writes in memory, and only push into the real tries if must (intermediate root or commit).

[holiman]

Regarding that last commit, here's the corresponding test if we want to use blockchain testing:
Should be placed in core/blockchain_test.go:

func TestDeleteCreateRevertTransition(t *testing.T) {

	var (
		aa = common.HexToAddress("0x000000000000000000000000000000000000aaaa")
		bb = common.HexToAddress("0x000000000000000000000000000000000000bbbb")
		// Generate a canonical chain to act as the main dataset
		engine = ethash.NewFaker()
		db     = rawdb.NewMemoryDatabase()

		// A sender who makes transactions, has some funds
		key, _  = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
		address = crypto.PubkeyToAddress(key.PublicKey)
		funds   = big.NewInt(1000000000)
		gspec   = &Genesis{
			Config: params.TestChainConfig,
			Alloc: GenesisAlloc{
				address: {Balance: funds},
				// The address 0xAAAAA selfdestructs if called
				aa: {
					// Code needs to just selfdestruct
					Code:    []byte{byte(vm.PC), 0xFF},
					Nonce:   1,
					Balance: big.NewInt(0),
				},
				// The address 0xBBBB send 1 wei to 0xAAAA, then reverts
				bb: {
					Code: []byte{
						byte(vm.PC),          // [0]
						byte(vm.DUP1),        // [0,0]
						byte(vm.DUP1),        // [0,0,0]
						byte(vm.DUP1),        // [0,0,0,0]
						byte(vm.PUSH1), 0x01, // [0,0,0,0,1] (value)
						byte(vm.PUSH2), 0xaa, 0xaa, // [0,0,0,0,1, 0xaaaa]
						byte(vm.GAS),
						byte(vm.CALL),
						byte(vm.REVERT),
					},
					Balance: big.NewInt(1),
				},
			},
		}
		genesis = gspec.MustCommit(db)
	)

	blocks, _ := GenerateChain(params.TestChainConfig, genesis, engine, db, 1, func(i int, b *BlockGen) {
		b.SetCoinbase(common.Address{1})
		// One transaction to AAAA
		tx, _ := types.SignTx(types.NewTransaction(0, aa,
			big.NewInt(0), 50000, big.NewInt(1), nil), types.HomesteadSigner{}, key)
		b.AddTx(tx)
		// One transaction to BBBB
		tx, _ = types.SignTx(types.NewTransaction(1, bb,
			big.NewInt(0), 100000, big.NewInt(1), nil), types.HomesteadSigner{}, key)
		b.AddTx(tx)

	})

	// Import the canonical chain
	diskdb := rawdb.NewMemoryDatabase()
	gspec.MustCommit(diskdb)

	chain, err := NewBlockChain(diskdb, nil, params.TestChainConfig, engine, vm.Config{
		Tracer: vm.NewJSONLogger(nil, os.Stdout), Debug: true,
	}, nil)
	if err != nil {
		t.Fatalf("failed to create tester chain: %v", err)
	}
	if n, err := chain.InsertChain(blocks); err != nil {
		t.Fatalf("block %d: failed to insert into chain: %v", n, err)
	}
}

This shouldn't be too hard to convert into a cross-client consensus blockchain test, cc @winsvega ?

[holiman]

Without the last commit, the test above yields

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x693c7b]

goroutine 28 [running]:
testing.tRunner.func1(0xc000106300)
	/usr/local/go/src/testing/testing.go:792 +0x387
panic(0x96cb60, 0xf34f20)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
github.com/ethereum/go-ethereum/core/state.(*StateDB).IntermediateRoot(0xc0003da120, 0xc0003ad401, 0x0, 0x0, 0x0, 0x0)
	/home/user/go/src/github.com/ethereum/go-ethereum/core/state/statedb.go:678 +0x12b
github.com/ethereum/go-ethereum/consensus/ethash.(*Ethash).FinalizeAndAssemble(0xc0000fe2a0, 0xb1f280, 0xc00013d3e0, 0xc0003b2b40, 0xc0003da120, 0xc00013d680, 0x2, 0x2, 0x0, 0x0, ...)
	/home/user/go/src/github.com/ethereum/go-ethereum/consensus/ethash/consensus.go:576 +0xe3
github.com/ethereum/go-ethereum/core.GenerateChain.func1(0x0, 0xc0003b4120, 0xc0003da120, 0x885a25897a359638, 0xb1f3a0, 0xc00013d3f0, 0xc0003da120)
	/home/user/go/src/github.com/ethereum/go-ethereum/core/chain_makers.go:216 +0x28e
github.com/ethereum/go-ethereum/core.GenerateChain(0xf7c380, 0xc0003b4120, 0xb21220, 0xc000

[winsvega]

It shouldn't. But what is the expected result you check in the test?

[holiman]

It shouldn't. But what is the expected result you check in the test?

No, I know it shouldn't, I just wanted to verify that the testcase did trigger the bug and that the last commit fixes it.

The expected result isn't really that important, I'm not sure at the moment :)

[winsvega]

Yes. If you put this contract code in the state test. Make a transaction to one or both transactions at the same code followed one by one as a subcall it must trigger the same behavior.

[winsvega]

You coming to Berlin at September? We could work together at the office

[holiman]

Yes. If you put this contract code in the state test. Make a transaction to one or both transactions at the same code followed one by one as a subcall it must trigger the same behavior.

No, I'm not so sure about that. The thing is that this triggers the intermediate state root calculation between the two transactoins. I don't think the same scenario could be constructed with only one transaction.
Yes, we can work in the office in Berlin

[holiman]

@karalabe I added the test here; https://github.com/holiman/go-ethereum/tree/blocktest . Feel free to pull that commit in if you want to

[holiman]

Looks good to me, but a question or two

[holiman]

Looks good to me (still). But I'd prefer if you also add that blockchain test defined here: #19953 (comment) . So it won't get lost, in case we want to convert it to a cross-client test later

[karalabe]

Pushed a new commit on top, impersonating you :P

[holiman]

Sweet, I'm basking in the fame and glory