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

feat: enable KromaZKTrie by default #78

Merged
merged 5 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cmd/geth/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func makeFullNode(ctx *cli.Context) (*node.Node, ethapi.Backend) {
}

cfg.Eth.CircuitParams = new(params.CircuitParams)
cfg.Eth.ExperimentalZkTree = ctx.Bool(utils.ExperimentalZkTrie.Name)
cfg.Eth.KromaZKTrie = ctx.Bool(utils.KromaZKTrie.Name)
if ctx.IsSet(utils.MaxTxsFlag.Name) {
maxTxs := ctx.Int(utils.MaxTxsFlag.Name)
cfg.Eth.CircuitParams.MaxTxs = &maxTxs
Expand Down
2 changes: 1 addition & 1 deletion cmd/geth/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ var (
// [kroma unsupported]
// utils.RollupHaltOnIncompatibleProtocolVersionFlag,
// utils.RollupSuperchainUpgradesFlag,
utils.ExperimentalZkTrie,
utils.KromaZKTrie,
configFileFlag,
}, utils.NetworkFlags, utils.DatabaseFlags)

Expand Down
7 changes: 4 additions & 3 deletions cmd/utils/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -1036,10 +1036,11 @@ Please note that --` + MetricsHTTPFlag.Name + ` must be set to start the server.
Category: flags.MetricsCategory,
}

ExperimentalZkTrie = &cli.BoolFlag{
Name: "zkstatetrie",
Usage: "use ZkMerkleStateTrie instead of ZkTrie in state. This is an experimental flag.",
KromaZKTrie = &cli.BoolFlag{
Name: "KromaZKTrie",
Usage: "use ZkMerkleStateTrie instead of ZkTrie in state.",
Category: flags.EthCategory,
Value: true,
}
)

Expand Down
4 changes: 2 additions & 2 deletions core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,12 @@ type CacheConfig struct {
SnapshotNoBuild bool // Whether the background generation is allowed
SnapshotWait bool // Wait for snapshot construction on startup. TODO(karalabe): This is a dirty hack for testing, nuke it

ExperimentalZkTrie bool // use ZkMerkleStateTrie instead of ZkTrie
KromaZKTrie bool // use ZkMerkleStateTrie instead of ZkTrie
}

// triedbConfig derives the configures for trie database.
func (c *CacheConfig) triedbConfig() *trie.Config {
config := &trie.Config{Preimages: c.Preimages, ExperimentalZkTrie: c.ExperimentalZkTrie}
config := &trie.Config{Preimages: c.Preimages, KromaZKTrie: c.KromaZKTrie}
if c.StateScheme == rawdb.HashScheme {
config.HashDB = &hashdb.Config{
CleanCacheSize: c.TrieCleanLimit * 1024 * 1024,
Comment on lines 151 to 162
Copy link

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [14-14]

The TODO comment regarding adding tests should be addressed to ensure the reliability and correctness of the code.

Would you like me to help generate tests for this functionality?

Expand Down
4 changes: 2 additions & 2 deletions core/state/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ type cachingDB struct {

// OpenTrie opens the main account trie at a specific root hash.
func (db *cachingDB) OpenTrie(root common.Hash) (Trie, error) {
if db.triedb.IsZkStateTrie() {
if db.triedb.IsKromaZK() {
return trie.NewZkMerkleStateTrie(root, db.triedb)
}
// [Scroll: START]
Expand All @@ -197,7 +197,7 @@ func (db *cachingDB) OpenTrie(root common.Hash) (Trie, error) {

// OpenStorageTrie opens the storage trie of an account.
func (db *cachingDB) OpenStorageTrie(stateRoot common.Hash, address common.Address, root common.Hash) (Trie, error) {
if db.triedb.IsZkStateTrie() {
if db.triedb.IsKromaZK() {
return trie.NewZkMerkleStateTrie(root, db.triedb)
}
// [Scroll: START]
Expand Down
Binary file not shown.
2 changes: 1 addition & 1 deletion eth/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
// [Scroll: START]
MPTWitness: config.MPTWitness,
// [Scroll: END]
ExperimentalZkTrie: config.ExperimentalZkTree,
KromaZKTrie: config.KromaZKTrie,
}
)
// Override the chain config with provided settings.
Expand Down
2 changes: 1 addition & 1 deletion eth/ethconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ type Config struct {
// [Scroll: END]
CircuitParams *params.CircuitParams

ExperimentalZkTree bool
KromaZKTrie bool
}

// CreateConsensusEngine creates a consensus engine for the given chain config.
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st
}

func (s *BlockChainAPI) newStateTrie(id *trie.ID, db *trie.Database) (state.Trie, error) {
if db.IsZkStateTrie() {
if db.IsKromaZK() {
Copy link

Choose a reason for hiding this comment

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

The condition if db.IsKromaZK() checks for a specific trie type but does not account for potential future trie types or configurations. Consider future-proofing this logic by either implementing a more flexible trie type handling mechanism or documenting the need for updates when new trie types are introduced.


The method newStateTrie uses a conditional check to determine the trie type but does not handle potential errors or unexpected trie types. It's recommended to add error handling for unsupported trie types to prevent runtime panics or undefined behavior.

+ if err != nil {
+   return nil, fmt.Errorf("unsupported trie type: %v", err)
+ }

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if db.IsKromaZK() {
if db.IsKromaZK() {
+ if err != nil {
+ return nil, fmt.Errorf("unsupported trie type: %v", err)
+ }

The use of IsKromaZK and IsZk methods to determine the trie type introduces tight coupling between the newStateTrie function and the specific trie implementations. Consider abstracting trie creation behind a factory pattern to improve modularity and facilitate easier addition of new trie types in the future.


In the newStateTrie method, there's a direct return in the conditional branches without any logging or metrics collection. For debugging and monitoring purposes, it might be beneficial to log the trie type being used or collect metrics on trie usage.

+ log.Debug("Creating new state trie", "type", "KromaZK")

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if db.IsKromaZK() {
if db.IsKromaZK() {
+ log.Debug("Creating new state trie", "type", "KromaZK")

The method newStateTrie does not validate the integrity or authenticity of the trie ID or root being passed to it. In a blockchain context, ensuring data integrity is crucial. Consider implementing additional checks or leveraging existing cryptographic validations to ensure the trie data's integrity before usage.

return trie.NewZkMerkleStateTrie(id.Root, db)
}
if db.IsZk() {
Expand Down
8 changes: 7 additions & 1 deletion miner/payload_building.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,13 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
// Setup the timer for terminating the process if SECONDS_PER_SLOT (12s in
// the Mainnet configuration) have passed since the point in time identified
// by the timestamp parameter.
endTimer := time.NewTimer(time.Second * 12)

// KROMA has a blocktime of 2 seconds, so we fixed it.
endTime := time.Second * 2
if w.chainConfig.Kroma == nil {
endTime = time.Second * 12
}
endTimer := time.NewTimer(endTime)

fullParams := &generateParams{
timestamp: args.Timestamp,
Expand Down
16 changes: 8 additions & 8 deletions trie/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type Config struct {
PathDB *pathdb.Config // Configs for experimental path-based scheme
Zktrie bool // use zktrie

ExperimentalZkTrie bool // use zktree
KromaZKTrie bool // use zktree
}

// HashDefaults represents a config for using hash-based scheme with
Expand Down Expand Up @@ -378,19 +378,19 @@ func (db *Database) Get(key []byte) ([]byte, error) {
return zdb.Get(key)
}

func (db *Database) IsZk() bool { return db.config.Zktrie }
func (db *Database) IsZkStateTrie() bool { return db.config.Zktrie && db.config.ExperimentalZkTrie }
func (db *Database) IsZk() bool { return db.config.Zktrie }
func (db *Database) IsKromaZK() bool { return db.config.Zktrie && db.config.KromaZKTrie }

func (db *Database) SetBackend(isZk bool) {
if db.config.Zktrie == isZk {
return
}
db.config = &Config{
Preimages: db.config.Preimages,
HashDB: db.config.HashDB,
PathDB: db.config.PathDB,
Zktrie: isZk,
ExperimentalZkTrie: db.config.ExperimentalZkTrie,
Preimages: db.config.Preimages,
HashDB: db.config.HashDB,
PathDB: db.config.PathDB,
Zktrie: isZk,
KromaZKTrie: db.config.KromaZKTrie,
}
if db.config.PathDB != nil {
if isZk {
Expand Down
5 changes: 3 additions & 2 deletions trie/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ func zkMerkleTreeNodeBlobFunctions(findBlobByHash func(key []byte) ([]byte, erro
return &merkleTreeIteratorLeafNode{
hash: hash,
blob: n.Data(),
key: zkt.ReverseByteOrder(n.Key),
key: BytesToZkIteratorKey(n.Key).Bytes(),
}, nil
}
return nil, nil
Expand Down Expand Up @@ -899,7 +899,7 @@ func zktrieNodeBlobFunctions(t *zktrie.ZkTrie) (
return &merkleTreeIteratorLeafNode{
hash: hash,
blob: node.Data(),
key: node.NodeKey.Bytes(),
key: BytesToZkIteratorKey(node.NodeKey[:]).Bytes(),
}, nil
}
return nil, nil
Expand Down Expand Up @@ -933,6 +933,7 @@ func (it *merkleTreeIterator) seek(path []byte) {
if len(path) == 0 {
return
}
path = zk.NewTreePathFromHashBig(common.BytesToHash(path))

for _, p := range path {
if parent, ok := it.stack[len(it.stack)-1].(*merkleTreeIteratorParentNode); ok {
Expand Down
26 changes: 26 additions & 0 deletions trie/iterator_key.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package trie

import (
zkt "github.com/kroma-network/zktrie/types"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/trie/zk"
)

// File for defining an iterator key and managing key <-> iterator key conversion functions.
//
// NodeIterator.LeafKey must satisfy the following characteristics
// ```
// preordertraversal(tree).filter(x -> x is leaf).map(x -> x.leafKey) == sort(leafKeys)
// ```
// Since Trie satisfies this condition, functions that utilize it do not require any additional code (e.g. snapsync).
// However, zktrie does not, so unless we introduce a new key concept, we need to modify the core source code quite a bit.
// Therefore, we introduced the concept of iteratorKey, which satisfies the above condition, and implemented functions to convert key and iteratorKey.

func BytesToZkIteratorKey(data []byte) common.Hash {
return common.BigToHash(zk.NewTreePathFromBytes(data).ToBigInt())
}

func ZkIteratorKeyToZkHash(key common.Hash) zkt.Hash {
return *zk.NewTreePathFromHashBig(key).ToZkHash()
}
4 changes: 2 additions & 2 deletions trie/iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ func TestMerkleTreeIterator(t *testing.T) {
db.Delete(zkt.ReverseByteOrder(it.Hash().Bytes()))
if it.Leaf() {
leafCount++
leafPath := zk.NewTreePathFromHash(common.BytesToHash(it.LeafKey()))
leafPath := zk.NewTreePathFromHashBig(common.BytesToHash(it.LeafKey()))
if !bytes.Equal(it.Path(), leafPath[:len(it.Path())]) {
t.Errorf("incorrect tree path. iterator path : %v, leaf path %v", it.Path(), leafPath)
}
Expand Down Expand Up @@ -670,7 +670,7 @@ func TestMerkleTreeIterator(t *testing.T) {
var inputs []*kvs
for _, data := range testdata1 {
hash := zk.MustNewSecureHash(common.LeftPadBytes([]byte(data.k), 32))
inputs = append(inputs, &kvs{k: string(zk.NewTreePathFromZkHash(*hash)), v: data.v})
inputs = append(inputs, &kvs{k: string(BytesToZkIteratorKey(hash[:]).Bytes()), v: data.v})
}
slices.SortFunc(inputs, func(a, b *kvs) int { return bytes.Compare([]byte(a.k), []byte(b.k)) })
for idx := 0; idx < len(inputs); idx++ {
Expand Down
2 changes: 1 addition & 1 deletion trie/merkle_trie.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ type MerkleStateTrie interface {
}

func NewMerkleStateTrie(id *ID, db *Database) (MerkleStateTrie, error) {
if db.IsZkStateTrie() {
if db.IsKromaZK() {
return NewZkMerkleStateTrie(id.Root, db)
}
if db.IsZk() {
Expand Down
9 changes: 5 additions & 4 deletions trie/zk/merkle_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (t *MerkleTree) MustDelete(key []byte) {
// mt.ImportDumpedLeafs), but this will lose all the Root history of the MerkleTree
func (t *MerkleTree) Delete(key []byte) error {
node, path, pathNodes := t.rootNode, t.newTreePath(key), *new([]*ParentNode)
for _, p := range path {
for lvl, p := range path {
switch n := node.(type) {
case *ParentNode:
pathNodes = append(pathNodes, n)
Expand All @@ -261,7 +261,7 @@ func (t *MerkleTree) Delete(key []byte) error {
case *EmptyNode:
return trie.ErrKeyNotFound
case *HashNode:
return trie.ErrKeyNotFound
return fmt.Errorf("Delete: encounter hash node. level %d, path %v", lvl, path[:lvl])
default:
return trie.ErrInvalidNodeFound
}
Expand Down Expand Up @@ -336,7 +336,8 @@ func (t *MerkleTree) Prove(key []byte, writeNode func(TreeNode) error) error {
return err
}
node := t.rootNode
for _, p := range t.newTreePath(key) {
path := t.newTreePath(key)
for lvl, p := range path {
// TODO: notice here we may have broken some implicit on the proofDb:
// the key is not keccak(value) and it even can not be derived from the value by any means without an actual decoding
if err := writeNode(node); err != nil {
Expand All @@ -350,7 +351,7 @@ func (t *MerkleTree) Prove(key []byte, writeNode func(TreeNode) error) error {
case *EmptyNode:
return nil
case *HashNode:
return nil
return fmt.Errorf("Prove: encounter hash node. level %d, path %v", lvl, path[:lvl])
default:
return trie.ErrInvalidNodeFound
}
Expand Down
27 changes: 27 additions & 0 deletions trie/zk/tree_path.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package zk

import (
"math/big"

zkt "github.com/kroma-network/zktrie/types"

"github.com/ethereum/go-ethereum/common"
Expand All @@ -14,6 +16,22 @@ const (
right = byte(1)
)

func NewTreePathFromHashBig(hash common.Hash) TreePath {
return NewTreePathFromBig(hash.Big(), len(hash)*8)
}

func NewTreePathFromBig(key *big.Int, maxLevel int) TreePath {
result := make([]byte, maxLevel)
for i := 0; i < maxLevel; i++ {
bit := new(big.Int).Mod(key, common.Big2)
if bit.Uint64() == 1 {
result[maxLevel-1-i] = byte(bit.Int64())
}
key.Rsh(key, 1)
}
return result
}

func NewTreePathFromHash(hash common.Hash) TreePath {
return NewTreePathFromBytesAndMaxLevel(zkt.ReverseByteOrder(hash[:]), len(hash)*8)
}
Expand Down Expand Up @@ -65,3 +83,12 @@ func (p TreePath) toHexBytes() []byte {
}
return bytes
}

func (p TreePath) ToBigInt() *big.Int {
result := new(big.Int)
lastIdx := len(p) - 1
for i, b := range p {
result.SetBit(result, lastIdx-i, uint(b))
}
return result
}
Loading