Skip to content

Conversation

@txhsl
Copy link
Contributor

@txhsl txhsl commented Oct 22, 2025

Ready for review. The activation of Testnet Cancun and Prague are both scheduled to timestamp 1761600000, which is 2025-10-28T05:20:00+08:00, about 133 hours from now on.

INFO [10-22|11:04:13.978] Ready for fork activation                fork=Prague date="27 Oct 25 21:20 UTC" remaining=130h15m46s timestamp=1,761,600,000
INFO [10-22|11:04:17.735] Imported new chain segment               number=5,636,524 hash=98cda2..07bb1b blocks=2048 txs=23   mgas=5.399   elapsed=4.770s      mgasps=1.132  age=18m59s    snapdiffs=1.57MiB    triedirty=0.00B
INFO [10-22|11:04:17.735] Skipping dBFT block callback due to sync "block index"=5,636,524 "dbft index"=1,876,205
INFO [10-22|11:04:18.009] Imported new chain segment               number=5,636,652 hash=476e2f..25b023 blocks=128  txs=3    mgas=0.826   elapsed=273.718ms   mgasps=3.018  snapdiffs=1.57MiB    triedirty=0.00B
INFO [10-22|11:04:18.010] Skipping dBFT block callback due to sync "block index"=5,636,652 "dbft index"=1,876,205
INFO [10-22|11:04:20.314] Imported new chain segment               number=5,636,654 hash=aef505..0e8bf2 blocks=2    txs=0    mgas=0.000   elapsed=12.711ms    mgasps=0.000  snapdiffs=1.57MiB    triedirty=0.00B
INFO [10-22|11:04:20.315] Skipping dBFT block callback due to sync "block index"=5,636,654 "dbft index"=1,876,205
INFO [10-22|11:04:20.316] Commit new sealing work                  number=5,636,655 sealhash=a46d3b..b4e1af txs=0    gas=0 fees=0 elapsed="544.88µs"
INFO [10-22|11:04:20.316] New block in the chain                   "dbft index"=1,876,205 "chain index"=5,636,654 hash=0xaef505566c473c098f235a26d2bd967e24b76f56278e142f58a9c7cba50e8bf2 "parent hash"=0x0ff0eef663218003ae7416e8314f5bed0fada2127a9f5e8d9c07c54d1664e1e3 primary=1 coinbase=0x1212000000000000000000000000000000000003 "mix digest"=0x4ca092e886297329440349fcff74ced5af9cefa9116e78bc72161d81d426d9da
INFO [10-22|11:04:20.317] DKG info                                 roundNumber=54 epochStartHeight=5,624,640 epochDuration=60480 sharePeriodDuration=2880 consensusList="[0x7dE5b7c69b344bb4e75A39442594753AB1C67078 0x05f1167317c9274FEc85D557c0aDb57f318a3A54 0x379ddafFfaA57d87e4CcFB8C72015C1dD105a30E 0x84a32405966791d077811d4E9f21B43b1E7dd911 0xaea4D663A7a67849056c72E5F1612F67c5f3BC55 0xd7831Da24B63a0B16423Fb178E6Fb6799b82D2b0 0x77c6a598E577a507288b14D6Aa976776F519B974]"
INFO [10-22|11:04:20.348] Imported new chain segment               number=5,636,655 hash=67be1e..40efbb blocks=1    txs=0    mgas=0.000   elapsed=4.806ms     mgasps=0.000  snapdiffs=1.57MiB    triedirty=0.00B
INFO [10-22|11:04:20.349] Commit new sealing work                  number=5,636,656 sealhash=33d6f4..f5f396 txs=0    gas=0 fees=0 elapsed="335.83µs"
INFO [10-22|11:04:20.403] Fetching latest sealing proposal         "desired number"=5,636,655
INFO [10-22|11:04:20.404] New chain segment detected               "dBFT latest block index"=5,636,654 "sealing proposal index"=5,636,656
INFO [10-22|11:04:20.415] DKG task watcher                         currentHeight=5,636,655 watchListLength=0
INFO [10-22|11:04:20.517] Sealing proposal updated                 number=5,636,656 sealhash=33d6f4..f5f396 "parent hash"=67be1e..40efbb txs=0
2025-10-22T11:04:20.517Z        INFO    initializing dbft       {"height": 5636656, "view": 0, "index": 2, "role": "Backup"}

@txhsl txhsl marked this pull request as draft October 22, 2025 04:00
@txhsl txhsl marked this pull request as ready for review October 22, 2025 07:54
@txhsl txhsl requested review from AnnaShaleva and qianhh and removed request for qianhh October 22, 2025 07:54
@txhsl txhsl added this to the v0.5.0 milestone Oct 22, 2025
Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

Let me also check the consensus part before the release.

@AnnaShaleva
Copy link
Member

I get the ERROR[10-22|13:26:04.567] Head block is not reachable error while trying to initialize testnet node's DB from scratch:

anna@kiwi:~/Documents/GitProjects/bane-labs/go-ethereum$ ./build/bin/geth init --datadir ./t3 ./config/genesis_testnet.json 
INFO [10-22|13:26:04.529] Maximum peer count                       ETH=50 total=50
INFO [10-22|13:26:04.531] Smartcard socket not found, disabling    err="stat /run/pcscd/pcscd.comm: no such file or directory"
INFO [10-22|13:26:04.537] Set global gas cap                       cap=50,000,000
INFO [10-22|13:26:04.538] Initializing the KZG library             backend=gokzg
INFO [10-22|13:26:04.539] Defaulting to pebble as the backing database
INFO [10-22|13:26:04.539] Allocated cache and file handles         database=/home/anna/Documents/GitProjects/bane-labs/go-ethereum/t3/geth/chaindata cache=16.00MiB handles=16
INFO [10-22|13:26:04.567] Opened ancient database                  database=/home/anna/Documents/GitProjects/bane-labs/go-ethereum/t3/geth/chaindata/ancient/chain readonly=false
INFO [10-22|13:26:04.567] State schema set to default              scheme=path
ERROR[10-22|13:26:04.567] Head block is not reachable
INFO [10-22|13:26:04.589] Opened ancient database                  database=/home/anna/Documents/GitProjects/bane-labs/go-ethereum/t3/geth/chaindata/ancient/state readonly=false
INFO [10-22|13:26:04.590] Initialized path database                cache=16.00MiB buffer=64.00MiB history=90000
INFO [10-22|13:26:04.590] Writing custom genesis block
INFO [10-22|13:26:04.609] Successfully wrote genesis state         database=chaindata hash=221f7d..9beb71

It wasn't like that in 1.13+, and looks like it's related to the chain freeze. Although DB is initialized in the end. Is it expected behaviour?

@txhsl
Copy link
Contributor Author

txhsl commented Oct 22, 2025

It wasn't like that in 1.13+, and looks like it's related to the chain freeze. Although DB is initialized in the end. Is it expected behaviour?

I have to say yes, and our client still works with this error log. I'm also syncing the TestNet, very close to the latest.

@AnnaShaleva
Copy link
Member

AnnaShaleva commented Oct 22, 2025

Another note that I have: all our existing nodes use statescheme=hash scheme for DB (and it's correct since all our nodes are full). However, if we initialize new node from scratch with the updated binary, the default scheme used for initialization is path (it's a change in the original Geth behaviour). So starting NeoX node from scratch will fail with the following error:

anna@kiwi:~/Documents/GitProjects/bane-labs/go-ethereum$ cat ./t3/node.log 
INFO [10-22|13:39:37.170] Maximum peer count                       ETH=30 total=30
INFO [10-22|13:39:37.172] Smartcard socket not found, disabling    err="stat /run/pcscd/pcscd.comm: no such file or directory"
INFO [10-22|13:39:37.190] Enabling recording of key preimages since archive mode is used
WARN [10-22|13:39:37.190] Disabled transaction unindexing for archive node
WARN [10-22|13:39:37.190] Forcing hash state-scheme for archive mode
INFO [10-22|13:39:37.190] Set global gas cap                       cap=50,000,000
INFO [10-22|13:39:37.191] Initializing the KZG library             backend=gokzg
INFO [10-22|13:39:37.192] Allocated trie memory caches             clean=307.00MiB dirty=0.00B
INFO [10-22|13:39:37.192] Using pebble as the backing database
INFO [10-22|13:39:37.192] Allocated cache and file handles         database=/home/anna/Documents/GitProjects/bane-labs/go-ethereum/t3/geth/chaindata cache=512.00MiB handles=524,288
INFO [10-22|13:39:37.205] Opened ancient database                  database=/home/anna/Documents/GitProjects/bane-labs/go-ethereum/t3/geth/chaindata/ancient/chain readonly=false
Fatal: Failed to register the Ethereum service: incompatible state scheme, stored: path, provided: hash

So starting from this version if anyone want to initialize a new database, an explicit --state.scheme=hash flag should be added to the geth init command. I think that this should be mentioned in README and in the current CHANGELOG.

@txhsl
Copy link
Contributor Author

txhsl commented Oct 22, 2025

So starting from this version if anyone want to initialize a new database, an explicit --state.scheme=hash flag should be added to the geth init command. I think that this should be mentioned in README and in the current CHANGELOG.

The case is a bit complicated, archive nodes are now enforced to hash, but will be allowed to path in the future. And snap nodes are set default to be path, while also allowed to be hash.

It depends on the Geth implementation, but yes, we can add it somewhere.

@txhsl txhsl force-pushed the rel-0.5.0 branch 2 times, most recently from 5254ed0 to 8860fa4 Compare October 23, 2025 03:32
@AnnaShaleva
Copy link
Member

There's one thing in #499 I have doubts about:

// Only take legacy pool transactions.
legacyTxs := make(types.Transactions, 0, len(dbftBlock.transactions))
for _, tx := range dbftBlock.transactions {
if tx.Type() != types.BlobTxType {
legacyTxs = append(legacyTxs, tx)
}
}
errs := c.staticPool.Add(legacyTxs, false)

@txhsl, what is the desired behaviour, should we accept blob transactions?

If we prohibit blob transactions, then they should be excluded from the miner's proposal at the PrepareRequest construction level, so that the rest of block fields were aligned with an actual transaction list. Then, at the PrepareRequest/PreBlock verification level if there's a blob transaction in the list, then the current proposal should be considered as invalid and CV should happen. The same logic is relevant for decrypted transactions.

If we allow blob transactions, then I suppose we should have some verification logic for them at the consensus level.

@txhsl
Copy link
Contributor Author

txhsl commented Oct 23, 2025

@txhsl, what is the desired behaviour, should we accept blob transactions?

If we can, then we should. I prefer to accept all compatible features from Ethereum. Only blob pool can handle blob transactions, and the state of these two pools are supposed to be independent. So blob transactions are skipped here.

	// ErrAlreadyReserved is returned if the sender address has a pending transaction
	// in a different subpool. For example, this error is returned in response to any
	// input transaction of non-blob type when a blob transaction from this sender
	// remains pending (and vice-versa).
	ErrAlreadyReserved = errors.New("address already reserved")

If we prohibit blob transactions, then they should be excluded from the miner's proposal at the PrepareRequest construction level, so that the rest of block fields were aligned with an actual transaction list. Then, at the PrepareRequest/PreBlock verification level if there's a blob transaction in the list, then the current proposal should be considered as invalid and CV should happen. The same logic is relevant for decrypted transactions.

It sounds different, but I think somehow similar to the current implementation. Blob transactions are ignored in PreBlock verification, because they can't and don't need to be verified by legacy pool.

Blob transactions and legacy pool transactions are mixed in the list, separating them and then putting them back together seems redundant.

If we allow blob transactions, then I suppose we should have some verification logic for them at the consensus level.

Yes, we should verify them. IMO, they are verified within ValidateBody() and ValidateState(). Any other logic should we apply?

@AnnaShaleva
Copy link
Member

Good, then everything is OK.

Any other logic should we apply?

Probably not, from what I saw.

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

LGTM.

@txhsl txhsl merged commit c44992a into bane-main Oct 23, 2025
2 checks passed
@txhsl txhsl deleted the rel-0.5.0 branch October 23, 2025 08:46
@AnnaShaleva AnnaShaleva mentioned this pull request Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants