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

trie: avoid endianness conversion in GetTreeKey #140

Merged
merged 4 commits into from
Nov 24, 2022
Merged

trie: avoid endianness conversion in GetTreeKey #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42306ea
trie/utils: add concrete expected value in trie key generation test
jsign Nov 21, 2022
b1f131f
mod: update to latest go-verkle
jsign Nov 21, 2022
a5ef57e
trie/utils: avoid endianness conversions
jsign Nov 21, 2022
619bcfb
apply review changes & update to official go-verkle version
jsign Nov 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmd/geth/verkle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func checkChildren(root verkle.VerkleNode, resolver verkle.NodeResolverFn) error
switch node := root.(type) {
jsign marked this conversation as resolved.
Show resolved Hide resolved
case *verkle.InternalNode:
for i, child := range node.Children() {
childC := child.ComputeCommitment().Bytes()
childC := child.Commitment().Bytes()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mentioning something that might be off-topic for this PR, but I wonder if it might be interesting at some point to unify Commitment() and Commit().

Mostly because Commitment() panics if there's no value in the cache, and as a client, it might not be entirely obvious when that invariant holds.

So, something like a unique Commit() method that checks the cache, and whenever some action touches that node, it sets that to nil again, so on the next Commit() that is re-computed.

I feel you intentionally created these Commit() and Commitment() APIs, so there is probably a good reason I can be missing. Maybe it's useful to access the previous commitment value even after touching the node.

I'm just surfacing some personal uncertainty I had while deciding between Commit() and Commitment() since it wasn't entirely obvious and could produce a panic.

Copy link
Owner

Choose a reason for hiding this comment

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

It used to be the same, but at some point I found it useful to distinguish between a function that enforces the recomputation of the commitment and one that just returns it if it's available and panics if it's not.

I have a big refactor in the pipe that reduces the need for this distinction, and it will make sense to do what you say when it's merged.


childS, err := resolver(childC[:])
if bytes.Equal(childC[:], zero[:]) {
Expand All @@ -87,7 +87,7 @@ func checkChildren(root verkle.VerkleNode, resolver verkle.NodeResolverFn) error
// depth is set to 0, the tree isn't rebuilt so it's not a problem
childN, err := verkle.ParseNode(childS, 0, childC[:])
if err != nil {
return fmt.Errorf("decode error child %x in db: %w", child.ComputeCommitment().Bytes(), err)
return fmt.Errorf("decode error child %x in db: %w", child.Commitment().Bytes(), err)
}
if err := checkChildren(childN, resolver); err != nil {
return fmt.Errorf("%x%w", i, err) // write the path to the erroring node
Expand Down Expand Up @@ -204,7 +204,7 @@ func expandVerkle(ctx *cli.Context) error {
root.Get(key, chaindb.Get)
}

if err := os.WriteFile("dump.dot", []byte(verkle.ToDot(root)), 0600); err != nil {
if err := os.WriteFile("dump.dot", []byte(verkle.ToDot(root)), 0o600); err != nil {
jsign marked this conversation as resolved.
Show resolved Hide resolved
log.Error("Failed to dump file", "err", err)
} else {
log.Info("Tree was dumped to file", "file", "dump.dot")
Expand Down
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/cespare/cp v0.1.0
github.com/cloudflare/cloudflare-go v0.14.0
github.com/consensys/gnark-crypto v0.4.1-0.20210426202927-39ac3d4b3f1f
github.com/crate-crypto/go-ipa v0.0.0-20220916134416-c5abbdbdf644
github.com/crate-crypto/go-ipa v0.0.0-20221111143132-9aa5d42120bc
github.com/davecgh/go-spew v1.1.1
github.com/deckarep/golang-set v1.8.0
github.com/docker/docker v1.6.2
Expand All @@ -23,7 +23,7 @@ require (
github.com/fjl/gencodec v0.0.0-20220412091415-8bb9e558978c
github.com/fjl/memsize v0.0.0-20190710130421-bcb5799ab5e5
github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff
github.com/gballet/go-verkle v0.0.0-20220902153445-097bd83b7732
github.com/gballet/go-verkle v0.0.0-20221122140954-75ceda26b7db
github.com/go-stack/stack v1.8.0
github.com/golang-jwt/jwt/v4 v4.3.0
github.com/golang/protobuf v1.5.2
Expand Down
10 changes: 4 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ github.com/consensys/gnark-crypto v0.4.1-0.20210426202927-39ac3d4b3f1f/go.mod h1
github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/crate-crypto/go-ipa v0.0.0-20220523130400-f11357ae11c7/go.mod h1:gFnFS95y8HstDP6P9pPwzrxOOC5TRDkwbM+ao15ChAI=
github.com/crate-crypto/go-ipa v0.0.0-20220916134416-c5abbdbdf644 h1:1BOsVjUetPH2Lqv71Dh6uKLVj9WKdDr5KY57KZBbsWU=
github.com/crate-crypto/go-ipa v0.0.0-20220916134416-c5abbdbdf644/go.mod h1:gFnFS95y8HstDP6P9pPwzrxOOC5TRDkwbM+ao15ChAI=
github.com/crate-crypto/go-ipa v0.0.0-20221111143132-9aa5d42120bc h1:mtR7MuscVeP/s0/ERWA2uSr5QOrRYy1pdvZqG1USfXI=
github.com/crate-crypto/go-ipa v0.0.0-20221111143132-9aa5d42120bc/go.mod h1:gFnFS95y8HstDP6P9pPwzrxOOC5TRDkwbM+ao15ChAI=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
github.com/dave/jennifer v1.2.0/go.mod h1:fIb+770HOpJ2fmN9EPPKOqm1vMGhB+TwXKMZhrIygKg=
Expand Down Expand Up @@ -136,8 +135,8 @@ github.com/garslo/gogen v0.0.0-20170306192744-1d203ffc1f61 h1:IZqZOB2fydHte3kUgx
github.com/garslo/gogen v0.0.0-20170306192744-1d203ffc1f61/go.mod h1:Q0X6pkwTILDlzrGEckF6HKjXe48EgsY/l7K7vhY4MW8=
github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff h1:tY80oXqGNY4FhTFhk+o9oFHGINQ/+vhlm8HFzi6znCI=
github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff/go.mod h1:x7DCsMOv1taUwEWCzT4cmDeAkigA5/QCwUodaVOe8Ww=
github.com/gballet/go-verkle v0.0.0-20220902153445-097bd83b7732 h1:AB7YjNrzlVHsYz06zCULVV2zYCEft82P86dSmtwxKL0=
github.com/gballet/go-verkle v0.0.0-20220902153445-097bd83b7732/go.mod h1:o/XfIXWi4/GqbQirfRm5uTbXMG5NpqxkxblnbZ+QM9I=
github.com/gballet/go-verkle v0.0.0-20221122140954-75ceda26b7db h1:YvtZfE11QEYWPjsQCyZLoZCGMsxJs9mTEbhF3MnM32Q=
github.com/gballet/go-verkle v0.0.0-20221122140954-75ceda26b7db/go.mod h1:DMDd04jjQgdynaAwbEgiRERIGpC8fDjx0+y06an7Psg=
github.com/getkin/kin-openapi v0.53.0/go.mod h1:7Yn5whZr5kJi6t+kShccXS8ae1APpYTW6yheSwk8Yi4=
github.com/getkin/kin-openapi v0.61.0/go.mod h1:7Yn5whZr5kJi6t+kShccXS8ae1APpYTW6yheSwk8Yi4=
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
Expand Down Expand Up @@ -552,7 +551,6 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211020174200-9d6173849985/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 h1:h+EGohizhe9XlX18rfpa8k8RAc5XyaeamM+0VHRd4lc=
golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
Expand Down
35 changes: 19 additions & 16 deletions trie/utils/verkle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,31 +63,35 @@ func GetTreeKey(address []byte, treeIndex *uint256.Int, subIndex byte) []byte {
var aligned [32]byte
address = append(aligned[:32-len(address)], address...)
}
var poly [5]fr.Element

poly[0].SetZero()
gballet marked this conversation as resolved.
Show resolved Hide resolved
// poly = [2+256*64, address_le_low, address_le_high, tree_index_le_low, tree_index_le_high]
var poly [5]fr.Element

// 32-byte address, interpreted as two little endian
// 16-byte numbers.
verkle.FromLEBytes(&poly[1], address[:16])
verkle.FromLEBytes(&poly[2], address[16:])

// little-endian, 32-byte aligned treeIndex
var index [32]byte
for i, b := range treeIndex.Bytes() {
index[len(treeIndex.Bytes())-1-i] = b
}
verkle.FromLEBytes(&poly[3], index[:16])
verkle.FromLEBytes(&poly[4], index[16:])
jsign marked this conversation as resolved.
Show resolved Hide resolved

cfg, _ := verkle.GetConfig()
// treeIndex must be interpreted as a 32-byte aligned little-endian integer.
// e.g: if treeIndex is 0xAABBCC, we need the byte representation to be 0xCCBBAA00...00.
// poly[3] = LE({CC,BB,AA,00...0}) (16 bytes), poly[4]=LE({00,00,...}) (16 bytes).
//
// To avoid unnecessary endianness conversions for go-ipa, we do some trick:
// - poly[3]'s byte representation is the same as the *top* 16 bytes (trieIndexBytes[16:]) of
// 32-byte aligned big-endian representation (BE({00,...,AA,BB,CC})).
// - poly[4]'s byte representation is the same as the *low* 16 bytes (trieIndexBytes[:16]) of
// the 32-byte aligned big-endian representation (BE({00,00,...}).
trieIndexBytes := treeIndex.Bytes32()
verkle.FromBytes(&poly[3], trieIndexBytes[16:])
verkle.FromBytes(&poly[4], trieIndexBytes[:16])

cfg := verkle.GetConfig()
ret := cfg.CommitToPoly(poly[:], 0)

// add a constant point
// add a constant point corresponding to poly[0]=[2+256*64].
ret.Add(ret, getTreePolyIndex0Point)

return PointToHash(ret, subIndex)

}

func GetTreeKeyAccountLeaf(address []byte, leaf byte) []byte {
Expand Down Expand Up @@ -187,14 +191,13 @@ func getTreeKeyWithEvaluatedAddess(evaluated *verkle.Point, treeIndex *uint256.I
verkle.FromLEBytes(&poly[3], index[:16])
verkle.FromLEBytes(&poly[4], index[16:])

cfg, _ := verkle.GetConfig()
cfg := verkle.GetConfig()
ret := cfg.CommitToPoly(poly[:], 0)

// add the pre-evaluated address
ret.Add(ret, evaluated)

return PointToHash(ret, subIndex)

}

func EvaluateAddressPoint(address []byte) *verkle.Point {
Expand All @@ -211,7 +214,7 @@ func EvaluateAddressPoint(address []byte) *verkle.Point {
verkle.FromLEBytes(&poly[1], address[:16])
verkle.FromLEBytes(&poly[2], address[16:])

cfg, _ := verkle.GetConfig()
cfg := verkle.GetConfig()
ret := cfg.CommitToPoly(poly[:], 0)

// add a constant point
Expand Down
11 changes: 9 additions & 2 deletions trie/utils/verkle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package utils

import (
"crypto/sha256"
"encoding/hex"
"math/big"
"math/rand"
"testing"
Expand All @@ -34,13 +35,19 @@ func TestGetTreeKey(t *testing.T) {
n := uint256.NewInt(1)
n = n.Lsh(n, 129)
n.Add(n, uint256.NewInt(3))
GetTreeKey(addr[:], n, 1)
tk := GetTreeKey(addr[:], n, 1)

got := hex.EncodeToString(tk)
exp := "f42f932f43faf5d14b292b9009c45c28da61dbf66e20dbedc2e02dfd64ff5a01"
if got != exp {
t.Fatalf("Generated trie key is incorrect: %s != %s", got, exp)
}
Comment on lines +38 to +44
Copy link
Collaborator Author

@jsign jsign Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I took the liberty to improve this test a bit to enforce an expected value.

I did this in the first commit of the PR before changing GetTreeKey(...) so I could have an anchor source of truth to know if the improved implementation was generating the same output as before. The branch generates the f42f9... value before the improvement.

This test still passing makes me more comfortable about the change (and future ones, if any). I don't know if you have some way to gain more confidence in some other branch or similar; it never hurts. :)

Copy link
Owner

Choose a reason for hiding this comment

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

yeah that makes sense, thank you for that.

}

func TestConstantPoint(t *testing.T) {
var expectedPoly [1]verkle.Fr

cfg, _ := verkle.GetConfig()
cfg := verkle.GetConfig()
verkle.FromLEBytes(&expectedPoly[0], []byte{2, 64})
expected := cfg.CommitToPoly(expectedPoly[:], 1)

Expand Down
11 changes: 4 additions & 7 deletions trie/verkle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ func (t *VerkleTrie) TryGetAccount(key []byte) (*types.StateAccount, error) {
ck, err := t.TryGet(ckkey[:])
if err != nil {
return nil, fmt.Errorf("updateStateObject (%x) error: %v", key, err)

}
acc.CodeHash = ck

Expand Down Expand Up @@ -220,11 +219,11 @@ func (trie *VerkleTrie) TryDelete(key []byte) error {
// Hash returns the root hash of the trie. It does not write to the database and
// can be used even if the trie doesn't have one.
func (trie *VerkleTrie) Hash() common.Hash {
return trie.root.ComputeCommitment().Bytes()
return trie.root.Commit().Bytes()
}

func nodeToDBKey(n verkle.VerkleNode) []byte {
ret := n.ComputeCommitment().Bytes()
ret := n.Commitment().Bytes()
return ret[:]
}

Expand Down Expand Up @@ -277,6 +276,7 @@ func (trie *VerkleTrie) Copy(db *Database) *VerkleTrie {
db: db,
}
}

func (trie *VerkleTrie) IsVerkle() bool {
return true
}
Expand Down Expand Up @@ -306,10 +306,7 @@ func DeserializeAndVerifyVerkleProof(serialized []byte, rootC *verkle.Point, key
if err != nil {
return fmt.Errorf("could not deserialize proof: %w", err)
}
cfg, err := verkle.GetConfig()
if err != nil {
return fmt.Errorf("could not get configuration %w", err)
}
cfg := verkle.GetConfig()
if !verkle.VerifyVerkleProof(proof, cis, indices, yis, cfg) {
return errInvalidProof
}
Expand Down
8 changes: 4 additions & 4 deletions trie/verkle_iterator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func newVerkleNodeIterator(trie *VerkleTrie, start []byte) NodeIterator {
return new(nodeIterator)
}
it := &verkleNodeIterator{trie: trie, current: trie.root}
//it.err = it.seek(start)
// it.err = it.seek(start)
return it
}

Expand Down Expand Up @@ -129,13 +129,13 @@ func (it *verkleNodeIterator) Error() error {

// Hash returns the hash of the current node.
func (it *verkleNodeIterator) Hash() common.Hash {
return it.current.ComputeCommitment().Bytes()
return it.current.Commit().Bytes()
}

// Parent returns the hash of the parent of the current node. The hash may be the one
// grandparent if the immediate parent is an internal node with no hash.
func (it *verkleNodeIterator) Parent() common.Hash {
return it.stack[len(it.stack)-1].Node.ComputeCommitment().Bytes()
return it.stack[len(it.stack)-1].Node.Commit().Bytes()
}

// Path returns the hex-encoded path to the current node.
Expand Down Expand Up @@ -188,7 +188,7 @@ func (it *verkleNodeIterator) LeafProof() [][]byte {
panic("LeafProof() called on an verkle node iterator not at a leaf location")
}

//return it.trie.Prove(leaf.Key())
// return it.trie.Prove(leaf.Key())
panic("not completely implemented")
}

Expand Down
12 changes: 6 additions & 6 deletions trie/verkle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestReproduceTree(t *testing.T) {
}

proof, Cs, zis, yis, _ := verkle.MakeVerkleMultiProof(root, append(presentKeys, absentKeys...), kv)
cfg, _ := verkle.GetConfig()
cfg := verkle.GetConfig()
if !verkle.VerifyVerkleProof(proof, Cs, zis, yis, cfg) {
t.Fatal("could not verify proof")
}
Expand All @@ -88,7 +88,7 @@ func TestReproduceTree(t *testing.T) {
t.Fatal(err)
}
t.Logf("serialized: %x", p)
t.Logf("tree: %s\n%x\n", verkle.ToDot(root), root.ComputeCommitment().Bytes())
t.Logf("tree: %s\n%x\n", verkle.ToDot(root), root.Commitment().Bytes())
}

func TestChunkifyCodeTestnet(t *testing.T) {
Expand Down Expand Up @@ -294,7 +294,7 @@ func TestReproduceCondrieuStemAggregationInProofOfAbsence(t *testing.T) {
}

proof, Cs, zis, yis, _ := verkle.MakeVerkleMultiProof(root, append(presentKeys, absentKeys...), kv)
cfg, _ := verkle.GetConfig()
cfg := verkle.GetConfig()
if !verkle.VerifyVerkleProof(proof, Cs, zis, yis, cfg) {
t.Fatal("could not verify proof")
}
Expand All @@ -309,7 +309,7 @@ func TestReproduceCondrieuStemAggregationInProofOfAbsence(t *testing.T) {
t.Fatal(err)
}
t.Logf("serialized: %x", p)
t.Logf("tree: %s\n%x\n", verkle.ToDot(root), root.ComputeCommitment().Bytes())
t.Logf("tree: %s\n%x\n", verkle.ToDot(root), root.Commitment().Bytes())

t.Logf("%d", len(proof.ExtStatus))
if len(proof.ExtStatus) != 5 {
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestReproduceCondrieuPoAStemConflictWithAnotherStem(t *testing.T) {
}

proof, Cs, zis, yis, _ := verkle.MakeVerkleMultiProof(root, append(presentKeys, absentKeys...), kv)
cfg, _ := verkle.GetConfig()
cfg := verkle.GetConfig()
if !verkle.VerifyVerkleProof(proof, Cs, zis, yis, cfg) {
t.Fatal("could not verify proof")
}
Expand All @@ -356,7 +356,7 @@ func TestReproduceCondrieuPoAStemConflictWithAnotherStem(t *testing.T) {
t.Fatal(err)
}
t.Logf("serialized: %x", p)
t.Logf("tree: %s\n%x\n", verkle.ToDot(root), root.ComputeCommitment().Bytes())
t.Logf("tree: %s\n%x\n", verkle.ToDot(root), root.Commitment().Bytes())

t.Logf("%d", len(proof.ExtStatus))
if len(proof.PoaStems) != 0 {
Expand Down