bnb-chain / zkbnb Goto Github PK

Describe the Issue
A clear and concise description of what the bug is.

Your Environment

• OS Version: [e.g. MacOS]
• Which Browser and its version [e.g. chrome, safari]
• Node/Go Version [e.g. 15.2.0/1.15.2]

To Reproduce
Tell us how to reproduce this issue.
Ex. Steps to reproduce the behavior:

• 1. Go to '...'
• 2. Click on '....'
• 3. Scroll down to '....'
• 4. See error

If applicable, add screenshots to help explain your problem.

Expected behavior
Tell us what should happen

Describe the Issue
When doing perf test, there's a chance some pool_tx will be failed, might related to some timeout

Your Environment
single machine perf env

To Reproduce
deploy zkbnb, run stress-test at tps 400

Expected behavior
All pool_tx should be at status 2

Describe the Issue
I'm doing a perf test, when I set block size to 200, tps to 400, will see this error, pool tx will not increase.

stress-test: 29500: internal server error

6|committe | {"@timestamp":"2022-11-10T13:16:22.965Z","caller":"committer/committer.go:182","content":"apply pool tx ID: 29724 failed, err 21103: invalid nonce ","level":"error"}

7|witness | {"@timestamp":"2022-11-10T13:20:47.365Z","caller":"witness/witness.go:34","content":"failed to generate block witness, failed to construct block witness, block:14, err: state root doesn't match","level":"error"}

Your Environment

• OS Version:

NAME="Amazon Linux"
VERSION="2"
ID="amzn"
ID_LIKE="centos rhel fedora"
VERSION_ID="2"
PRETTY_NAME="Amazon Linux 2"
ANSI_COLOR="0;33"
CPE_NAME="cpe:2.3:o:amazon:amazon_linux:2"
HOME_URL="https://amazonlinux.com/"

• Which Browser and its version [e.g. chrome, safari]
• Node/Go Version : go1.18.6 linux/amd64 node: v16.18.1

To Reproduce

• 1. run deploy-local.sh, deploy contracts to BSC testnet
• 2. run script in stress-test:

#!/usr/bin/env bash

BASEDIR=$(dirname "$0")
DEPLOY_PATH=/zkbnb
KEY_PATH="${BASEDIR}/keyfile/key.json"
BLOCK_SIZE=200

# 1. generate accounts
FAKE_ACCOUNTS=1000
POSTGRES_DSN="host=localhost user=postgres password=ZkBNB@123 dbname=zkbnb port=5432 sslmode=disable"
ACCOUNT_BALANCE="1000000000000000000000000000000000"
ACCOUNT_OFFSET=4
[[ -f ${KEY_PATH} ]] && mv $KEY_PATH ${KEY_PATH}.`date +%Y%m%d%H%M%S`
go run main.go -account_num ${FAKE_ACCOUNTS} -dsn "${POSTGRES_DSN}" -key_path ${KEY_PATH} -account_offset ${ACCOUNT_OFFSET} -balance "${ACCOUNT_BALANCE}"

# 2. recovery accounts
pm2 delete committer
pm2 delete witness
cd ${DEPLOY_PATH}/zkbnb
go run ./cmd/zkbnb/main.go tree recovery \
          --config ./service/committer/etc/config.yaml \
            -batch 5000 --height 1 --service committer
go run ./cmd/zkbnb/main.go tree recovery \
          --config ./service/committer/etc/config.yaml \
            -batch 5000 --height 1 --service witness

# 3. restart committer & witness
sed -i -e "s/OptionalBlockSizes.*/OptionalBlockSizes: [${BLOCK_SIZE}]/" ${DEPLOY_PATH}/zkbnb/service/committer/etc/config.yaml
cd ${DEPLOY_PATH}/zkbnb
pm2 start --name committer "./run_committer.sh"
pm2 start --name witness "./run_witness.sh"

• 3. run script in stress-test:

#!/usr/bin/env bash

BASEDIR=$(dirname "$0")
KEY_PATH=$BASEDIR/keyfile/key.json
TPS=400
go run $BASEDIR/test/transfer/main.go -endpoint "http://localhost:8888" -tps ${TPS} -account_file ${KEY_PATH}

• 4. See error

stress-test: 29500: internal server error

6|committe | {"@timestamp":"2022-11-10T13:16:22.965Z","caller":"committer/committer.go:182","content":"apply pool tx ID: 29724 failed, err 21103: invalid nonce ","level":"error"}

7|witness | {"@timestamp":"2022-11-10T13:20:47.365Z","caller":"witness/witness.go:34","content":"failed to generate block witness, failed to construct block witness, block:14, err: state root doesn't match","level":"error"}

Expected behavior
pending tx metrics should be a horizon line at 10000 which is the limit set in apiserver

Describe the Issue
See the block circuit here: bnb-chain/zkbnb-crypto#46

To Reproduce
N/A

If applicable, add screenshots to help explain your problem.

Expected behavior
Witness service constructs correct empty tx witness for block circuit as follows:

• when calling EmptyTx(stateRoot[]byte) function, pass new StateRoot which generated by applying previous non-empty tx.

Describe the Issue
The package service/apiserver/internal/types is missing when building the code from scratch.
Your Environment

• OS Version: MacOS
• Which Browser and its version chrome
• Node/Go Version go version go1.20.1

To Reproduce
-1. go mod tidy
-2 meet the error

github.com/bnb-chain/zkbnb/service/apiserver/internal/handler/account imports
        github.com/bnb-chain/zkbnb/service/apiserver/internal/types: no matching versions for query "latest"

If applicable, add screenshots to help explain your problem.

Expected behavior
There is no package with name of types in the codebase, should there be error when refactoring the code?

Describe the Issue
Genesis block can't be verified from last week.
Before last week, event we generate pk vk at 10 block size, we're able to verify the genesis block, but after last week, status of tx were stucked at 4(StatusCommitted).
Is it designed result? if not, should figure out the reason.

Your Environment

any perf env or local env

To Reproduce
deploy zkbnb at local, waiting for the genesis block being committed.
see if tx status changed to 5

Expected behavior
genesis block should be verified.

https://github.com/binance/zkmerkle-proof-of-solvency/releases/download/v1.0.2/verifier_linux_amd64

Describe the Issue
There is typo on some file name
Your Environment

• OS Version: [e.g. MacOS]
• Which Browser and its version [e.g. chrome, safari]
• Node/Go Version [e.g. 15.2.0/1.15.2]

To Reproduce
Tell us how to reproduce this issue.
Ex. Steps to reproduce the behavior:

• 1. Go to core/model package
     The file name is mft_metadata.go, while it should be nft_metadata.go instead.

If applicable, add screenshots to help explain your problem.

Expected behavior
Tell us what should happen

https://github.com/0xPolygonHermez/zkevm-prover/blob/main/README.md

Describe the Issue
For now, FullExitNft transaction only generate one owner account related and one nft related tx detail, missing creator account related tx detail which is needed by circuit, see bnb-chain/zkbnb-crypto#28

Your Environment
N/A

To Reproduce
Tell us how to reproduce this issue.
N/A

If applicable, add screenshots to help explain your problem.

Expected behavior
FullExitNft transaction should generate a valid nft creator account related tx detail when the tx is valid and generate a empty nft creator account related tx detail when the tx is invalid.

Describe the Issue
When GetChanges ,this will always return null change , we will not store the Changed AssetInfo into Kvrock.

// Returns slice of indexes of asset trees that were changned
func (c *AssetTreeCache) GetChanges() []int64 {
c.mainLock.Lock()
c.changesLock.Lock()
defer c.mainLock.Unlock()
defer c.changesLock.Unlock()
for _, key := range c.treeCache.Keys() {
tree, _ := c.treeCache.Peek(key)
if tree.(bsmt.SparseMerkleTree).LatestVersion()-tree.(bsmt.SparseMerkleTree).RecentVersion() > 1 {
c.changes[key.(int64)] = true
}
}
ret := make([]int64, 0, len(c.changes))
for key := range c.changes {
ret = append(ret, key)
}
return ret
}
As we can see from above codes, committer service uses 【tree.(bsmt.SparseMerkleTree).LatestVersion()-tree.(bsmt.SparseMerkleTree).RecentVersion() > 1】 which is not correct. The reason is that the LatestVersion only change after the tree committed.

Your Environment

N/A

To Reproduce

N/A

Expected behavior
Assign correct GetChanges logic by

Describe the Issue
When constructing assets witness for circuit, witness service use dummy asset info and merkle proof if there isn't enough assets count, the codes are as following:

// padding empty account asset
for assetCount < NbAccountAssetsPerAccount {
	cryptoAccount.AssetsInfo[assetCount] = cryptoTypes.EmptyAccountAsset(LastAccountAssetId)
	assetMerkleProof, err := w.assetTrees.Get(accountKey).GetProof(LastAccountAssetId)
	if err != nil {
		return accountRootBefore, accountsInfoBefore, merkleProofsAccountAssetsBefore, merkleProofsAccountBefore, err
	}
	merkleProofsAccountAssetsBefore[accountCount][assetCount], err = SetFixedAccountAssetArray(assetMerkleProof)
	if err != nil {
		return accountRootBefore, accountsInfoBefore, merkleProofsAccountAssetsBefore, merkleProofsAccountBefore, err
	}
	assetCount++
}

As we can see from above codes, witness service uses LastAccountAssetId(65535) as the dummy asset leaf node, and assumes LastAccountAssetId is empty which is not correct. The reason is that zkbnb reuse asset tree to store nft offer ids and the offer id is specified by user, so there is high probability that the LastAccountAssetId leaf node will be used. Although there exists dummy accounts and nft witness, the last leaf node of account tree and nft tree is used with very small probability.

Your Environment

N/A

To Reproduce

N/A

Expected behavior
Assign correct LastAccountAssetId asset leaf node to witness

Describe the Issue

blockSize over 300 can't be sent to L1.
if test on testnet, set OptionalBlockSizes to 300, we can generate, but can't commit to L1 by sender.
Should dig into and find why input is so big, can we reduce them?

│4|sender   | {"@timestamp":"2022-11-09T12:31:12.480Z","caller":"sender/sender.go:33","content":"failed to rollup block, failed to send commit tx, errL oversized data:","level":"error"}

Your Environment

To Reproduce

Expected behavior
commit new block on L1