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.

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

Unexpected behavior for StateDB.IntermediateRoot and StateDB.Copy #28176

Closed
wuestholz opened this issue Sep 21, 2023 · 5 comments
Closed

Unexpected behavior for StateDB.IntermediateRoot and StateDB.Copy #28176

wuestholz opened this issue Sep 21, 2023 · 5 comments
Labels

Comments

@wuestholz
Copy link
Contributor

wuestholz commented Sep 21, 2023

System information

Geth version: v1.13.1

Expected behaviour

When I run the following code snippet, I don't expect a panic:

db := rawdb.NewMemoryDatabase()
st1, _ := state.New(types.EmptyRootHash, state.NewDatabaseWithConfig(db, &trie.Config{Preimages: true}), nil)

addrA := common.HexToAddress("0xaaaaaaaa")
bal, _ := math.ParseBig256("0xfffffffffffffffffffffff")
st1.SetBalance(addrA, bal)
var nonceA uint64 = 0
st1.SetNonce(addrA, nonceA)
st1.SetCode(addrA, []byte{})
addrB := common.HexToAddress("0xbbbbbbbb")
var nonceB uint64 = 1

st1Root, comErr1 := st1.Commit(0, false)
if comErr1 != nil {
	panic(comErr1)
}
st2, newErr1 := state.New(st1Root, st1.Database(), nil)
if newErr1 != nil {
	panic(newErr1)
}

st3 := st2

st3.SetNonce(addrB, nonceB)

skipIntermediateRoot := false // Setting this to true "fixes it".
if !skipIntermediateRoot {
	st3.IntermediateRoot(false)
}

var st4 *state.StateDB
skipCopy := false // Setting this to true "fixes it".
if skipCopy {
	st4 = st3
} else {
	st4 = st3.Copy()
}

st4Root, comErr2 := st4.Commit(0, false)
if comErr2 != nil {
	panic(comErr2)
}
st5, newErr2 := state.New(st4Root, st4.Database(), nil)
if newErr2 != nil {
	panic(newErr2)
}

if st5.GetNonce(addrB) != nonceB {
	panic("nonce of account B does not match")
}

openTr, openErr := st5.Database().OpenTrie(st4Root)
if openErr != nil {
	panic(openErr)
}
nodeIt, nodeItErr := openTr.NodeIterator(nil)
if nodeItErr != nil {
	panic(nodeItErr)
}
it := trie.NewIterator(nodeIt)
foundB := false
for it.Next() {
	key := openTr.GetKey(it.Key)
	if key == nil {
		continue
	}
	addr := common.BytesToAddress(key)
	if addr == addrB {
		foundB = true
	}
}
if !foundB {
	panic("account B was not found")
}

Actual behaviour

The code produces a panic ("account B was not found").

I don't understand why addrB is not found when iterating over the accounts of the final state (st5).

I found two "fixes", but I don't understand why they work: (1) setting skipCopy to true or alternatively (2) setting skipIntermediateRoot to true.

Is this the intended behavior?

Steps to reproduce the behaviour

Run the above code snippet with v1.13.1.

@holiman
Copy link
Contributor

holiman commented Apr 24, 2024

I don't understand why addrB is not found when iterating over the accounts of the final state (st5).

It's because key is the hash of the address.

	fmt.Printf("B key: %x\n", crypto.Keccak256Hash(addrB[:]))

spits out

B key: 1a683dac29b95766fd4e8dca2f4df319c4572a3ca22a1d3882072e1ff7a59a1e

And if you print out the iteration:

	for it.Next() {
		fmt.Printf("key %x\n", it.Key)

Then you'll get

key 024c1dc1ea6078f4b2976e31344b32893177c88dfeb061d8bc1d5ca69f8fa6ee
key 1a683dac29b95766fd4e8dca2f4df319c4572a3ca22a1d3882072e1ff7a59a1e

@holiman holiman closed this as completed Apr 24, 2024
@wuestholz
Copy link
Contributor Author

wuestholz commented Apr 25, 2024

@holiman Thanks for taking a look! I'm aware that it.Key is the hash of the address. That's why the code uses openTr.GetKey(it.Key) to get the bytes of the address. Below, I've updated the code to the latest version of go-ethereum (v1.14.0). However, I'm still seeing the same unexpected behavior.

func Test(t *testing.T) {
	db := rawdb.NewMemoryDatabase()
	st1, _ := state.New(types.EmptyRootHash, state.NewDatabaseWithConfig(db, &triedb.Config{Preimages: true}), nil)

	addrA := common.HexToAddress("0xaaaaaaaa")
	bal := new(uint256.Int)
	_ = bal.SetFromHex("0xfffffffffffffffffffffff")
	st1.SetBalance(addrA, bal, tracing.BalanceChangeUnspecified)
	var nonceA uint64 = 0
	st1.SetNonce(addrA, nonceA)
	st1.SetCode(addrA, []byte{})
	addrB := common.HexToAddress("0xbbbbbbbb")
	var nonceB uint64 = 1

	st1Root, comErr1 := st1.Commit(0, false)
	if comErr1 != nil {
		panic(comErr1)
	}
	st2, newErr1 := state.New(st1Root, st1.Database(), nil)
	if newErr1 != nil {
		panic(newErr1)
	}

	st3 := st2

	st3.SetNonce(addrB, nonceB)

	skipIntermediateRoot := false // Setting this to true "fixes it".
	if !skipIntermediateRoot {
		st3.IntermediateRoot(false)
	}

	var st4 *state.StateDB
	skipCopy := false // Setting this to true "fixes it".
	if skipCopy {
		st4 = st3
	} else {
		st4 = st3.Copy()
	}

	st4Root, comErr2 := st4.Commit(0, false)
	if comErr2 != nil {
		panic(comErr2)
	}
	st5, newErr2 := state.New(st4Root, st4.Database(), nil)
	if newErr2 != nil {
		panic(newErr2)
	}

	if st5.GetNonce(addrB) != nonceB {
		panic("nonce of account B does not match")
	}

	openTr, openErr := st5.Database().OpenTrie(st4Root)
	if openErr != nil {
		panic(openErr)
	}
	nodeIt, nodeItErr := openTr.NodeIterator(nil)
	if nodeItErr != nil {
		panic(nodeItErr)
	}
	it := trie.NewIterator(nodeIt)
	foundB := false
	for it.Next() {
		addrHash := it.Key
		fmt.Printf("addrHash: %x\n", addrHash)
		addrBytes := openTr.GetKey(addrHash)
		if addrBytes == nil {
			continue
		}
		fmt.Printf("addrBytes: %x\n", addrBytes)
		addr := common.BytesToAddress(addrBytes)
		if addr == addrB {
			foundB = true
		}
	}
	if !foundB {
		panic("account B was not found")
	}
}

I see the following output:

addrHash: 024c1dc1ea6078f4b2976e31344b32893177c88dfeb061d8bc1d5ca69f8fa6ee
addrBytes: 00000000000000000000000000000000aaaaaaaa
addrHash: 1a683dac29b95766fd4e8dca2f4df319c4572a3ca22a1d3882072e1ff7a59a1e
--- FAIL: Test (0.00s)
panic: account B was not found [recovered]
	panic: account B was not found

I don't understand why it fails to map the hash of the address 1a683dac29b95766fd4e8dca2f4df319c4572a3ca22a1d3882072e1ff7a59a1e to the address 0xbbbbbbbb.

However, I see the expected output when setting skipCopy or skipIntermediateRoot to true:

addrHash: 024c1dc1ea6078f4b2976e31344b32893177c88dfeb061d8bc1d5ca69f8fa6ee
addrBytes: 00000000000000000000000000000000aaaaaaaa
addrHash: 1a683dac29b95766fd4e8dca2f4df319c4572a3ca22a1d3882072e1ff7a59a1e
addrBytes: 00000000000000000000000000000000bbbbbbbb
--- PASS: Test (0.00s)

Can you explain this difference in behavior?

@holiman
Copy link
Contributor

holiman commented Apr 25, 2024

So, you can skip everything after openTr, openErr := st5.Database().OpenTrie(st4Root), and do:

	openTr, openErr := st5.Database().OpenTrie(st4Root)
	if openErr != nil {
		panic(openErr)
	}
	fmt.Printf("A: %x\n", openTr.GetKey(crypto.Keccak256(addrA[:])))
	fmt.Printf("B: %x\n", openTr.GetKey(crypto.Keccak256(addrB[:])))

It will show you that the preimage for address is not present.

A: 00000000000000000000000000000000aaaaaaaa
B: 

So it's not that account B is not found when iterating the trie (it is), it's that the hash cannot be reversed. I'm not 100% sure yet why that is.

@holiman
Copy link
Contributor

holiman commented Apr 25, 2024

THe preimage for account A was stored to db during the Commit, but addr b hasn't been part of any commit.

@wuestholz
Copy link
Contributor Author

@holiman Thanks for the quick reply! There is a second commit after the state update for addrB (st4Root, comErr2 := st4.Commit(0, false)). I would have expected that this commit will include the preimage for addrB. Could you explain why that doesn't happen? Why does skipCopy or skipIntermediateRoot change the behavior of the second commit? For instance, does Copy not copy the preimages?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants
@wuestholz @holiman and others