Skip to content
This repository was archived by the owner on Feb 17, 2025. It is now read-only.

Feature/#2429 fork id improvement#2450

Merged
ARR552 merged 13 commits into
release/v0.2.6from
feature/#2429_forkID_improvement
Aug 24, 2023
Merged

Feature/#2429 fork id improvement#2450
ARR552 merged 13 commits into
release/v0.2.6from
feature/#2429_forkID_improvement

Conversation

@ARR552
Copy link
Copy Markdown
Contributor

@ARR552 ARR552 commented Aug 23, 2023

Closes #2429

What does this PR do?

This Pr adds a forkID table. If a component needs the forkID list goes to db instead of to read the info to L1. If the table is empty the synchronizer fills it until the current synced batch. During the sync process, new forkIDs are stored in db and loaded in memory.
*Important points: *
If the synchronization has not finished, do not start the aggregator because the currentForkID could be wrong.
If the node is running using microservices, other components needs to be restarted in order to be aware of the new forkID. The synchronizer is the only one that will realize that the forkID has changed. If all the components are running together, this is not a problem because the synchronizer will keep updated the memory variables shared by all the components.

Config changes:
[Etherman]
ForkIDChunkSize = 20000

Reviewers

@agnusmor
@ToniRamirezM
@joanestebanr

@ARR552 ARR552 added this to the v0.2.6 milestone Aug 23, 2023
@ARR552 ARR552 requested a review from tclemos as a code owner August 23, 2023 07:18
@ARR552 ARR552 self-assigned this Aug 23, 2023
@cla-bot cla-bot Bot added the cla-signed label Aug 23, 2023
@ARR552 ARR552 requested a review from arnaubennassar as a code owner August 23, 2023 07:47
@ARR552 ARR552 force-pushed the feature/#2429_forkID_improvement branch from 91ed540 to 2458269 Compare August 23, 2023 14:02
@ARR552 ARR552 force-pushed the feature/#2429_forkID_improvement branch from 2458269 to ee008ba Compare August 23, 2023 15:05
Comment thread cmd/run.go Outdated
Comment thread cmd/run.go
} else if len(forkIntervals) == 0 {
return []state.ForkIDInterval{}, fmt.Errorf("error: no forkID received. It should receive at least one, please check the configuration...")
}
forkIDIntervals = forkIntervals
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We are not storing this initial forkID in the db

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, we are. Why do you say we are not storing it?

Comment thread cmd/run.go
if err != nil && !errors.Is(err, state.ErrStateNotSynchronized) {
return []state.ForkIDInterval{}, fmt.Errorf("error checking lastL1BlockSynced. Error: %v", err)
}
if lastBlock != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can change this if/else block code for this one, since the if and else code are the same, since I understand also that we need to store the forkid interval in the case of the initial fork (genesis):

lastBlockNumber := genesisBlockNumber
if lastBlock != nil {
   lastBlockNumber := lastBlock.BlockNumber
}

log.Info("Getting forkIDs intervals. Please wait...")
// Read Fork ID FROM POE SC
forkIntervals, err := etherman.GetForks(ctx, genesisBlockNumber, lastBlockNumber)
if err != nil {
   return []state.ForkIDInterval{}, fmt.Errorf("error getting forks. Please check the configuration. Error: %v", err)
} else if len(forkIntervals) == 0 {
   return []state.ForkIDInterval{}, fmt.Errorf("error: no forkID received. It should receive at least one, please check the configuration...")
}
log.Info("Storing forkID intervals into db")
// Store forkIDs
dbTx, err := st.BeginStateTransaction(ctx)
if err != nil {
   return []state.ForkIDInterval{}, fmt.Errorf("error creating dbTx. Error: %v", err)
}
for _, f := range forkIntervals {
   err := st.AddForkID(ctx, f, dbTx)
   if err != nil {
   err = dbTx.Rollback(ctx)
   if err != nil {
      return []state.ForkIDInterval{}, fmt.Errorf("error when rollback dbTx. Error: %v", err)
   }
      return []state.ForkIDInterval{}, fmt.Errorf("error adding forkID to db. Error: %v", err)
   }
}
err = dbTx.Commit(ctx)
if err != nil {
   return []state.ForkIDInterval{}, fmt.Errorf("error committing dbTx. Error: %v", err)
}
forkIDIntervals = forkIntervals

Comment thread cmd/run.go Outdated
Comment thread config/environments/local/local.node.config.toml Outdated
@@ -0,0 +1,13 @@
-- +migrate Up
CREATE TABLE IF NOT EXISTS state.fork_id
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think is good practice to define always a primary key for the tables, in this case I understand the PK is fork_id

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't fully understand what you mean. As you can see the fork_id is defined as Primary key
fork_id BIGINT NOT NULL PRIMARY KEY,

Comment thread etherman/etherman.go Outdated
Comment thread state/forkid.go
Comment thread state/forkid.go
Comment thread test/operations/manager.go
@ARR552 ARR552 merged commit eb11e0f into release/v0.2.6 Aug 24, 2023
@ARR552 ARR552 deleted the feature/#2429_forkID_improvement branch August 24, 2023 09:03
@ARR552 ARR552 added the config label Aug 24, 2023
ToniRamirezM added a commit that referenced this pull request Aug 29, 2023
* fix null effective_percentage

* fix forkID calculation

* fix script

* generate json-schema + docs for node config file and network_custom

* fix unittest

* Hotfixv0.1.4 to v0.2.0 (#2255)

* Hotfix v0.1.4 to main (#2250)

* fix concurrent web socket writes

* fix eth_syncing

* fix custom trace internal tx call error handling and update prover

* add test to custom tracer depth issue; fix internal call error and gas used

* fix custom tracer for internal tx with error and no more steps after it

* remove debug code

* Make max grpc message size configurable  (#2179)

* make max grpc message size configurable

* fix state tests

* fix tests

* fix tests

* get SequencerNodeURI from SC if empty and not IsTrustedSequencer

* Optimize trace (#2183)

* optimize trace

* fix memory reading

* update docker image

* update prover image

* fix converter

* fix memory

* fix step memory

* fix structlogs

* fix structlogs

* fix structlogs

* fix structlogs

* fix structlogs

* fix structlogs

* fix structlogs

* fix structlogs

* update prover image

* fix struclogs

* fix memory size

* fix memory size

* fix memory size

* refactor memory resize

* refactor memory resize

* move log for the best fitting tx (#2192)

* fix load zkCounters from pool

* remove unnecessary log.info

* add custom tracer support to CREATES opcode without depth increase (#2213)

* logs

* fix getting stateroot from previous batch (GetWIPBatch)

* logs

* Fix GetWipBatch when previous last batch is a forced batch

* fix forcedBatch trusted state

* Revert "fix getting stateroot from previous batch (GetWIPBatch)"

This reverts commit 860f0e7.

* force GHA

* add pool limits (#2189)

* Hotfix/batch l2 data (#2223)

* Fix BatchL2Data

* Force GHA

* remove failed txs from the pool limit check (#2233)

* debug trace by batch number via external rpc requests (#2235)

* fix trace batch remote requests in parallel limitation (#2244)

* Added RPC.TraceBatchUseHTTPS config parameter

* fix executor version

---------

Co-authored-by: tclemos <thiago@polygon.technology>
Co-authored-by: tclemos <thiago@iden3.com>
Co-authored-by: Toni Ramírez <58293609+ToniRamirezM@users.noreply.github.com>
Co-authored-by: agnusmor <agnusmor@gmail.com>
Co-authored-by: agnusmor <100322135+agnusmor@users.noreply.github.com>
Co-authored-by: Thiago Coimbra Lemos <tclemos@users.noreply.github.com>

* fix test

* fix test

---------

Co-authored-by: tclemos <thiago@polygon.technology>
Co-authored-by: tclemos <thiago@iden3.com>
Co-authored-by: Toni Ramírez <58293609+ToniRamirezM@users.noreply.github.com>
Co-authored-by: agnusmor <agnusmor@gmail.com>
Co-authored-by: agnusmor <100322135+agnusmor@users.noreply.github.com>
Co-authored-by: Thiago Coimbra Lemos <tclemos@users.noreply.github.com>

* Effective GasPrice refactor+fixes (#2247)

* effective GasPrice refactor

* bugs fixes and finalizer tests fixes

* fix typo

* fix calculate effective gasprice percentage

* fix test gas price

* Fix/#2257 effective gas price receipt (#2258)

* effective gas price returned by the rpc in the receipt

* linter

* bugfix: fixing l2blocks timestamp for the fist batch (#2260)

* bugfix: fixing l2blocks timestamp for the fist batch

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* fix finalizer unit test

---------

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* add more comments, and removed fields PrivateKeyPath and PrivateKeyPassword from etherman.Config that are not in use

* add info to git action

* add info to git action

* fix github action

* updated comments

* updated comments

* Fix/#2263 gas used (#2264)

* fix fea2scalar and gas used

* suggestion

* fix fea2scalar

* suggestion

* Fix pending tx when duplicate nonce (#2270)

* fix pending tx when duplicate nonce

* set pool.transaction.failed_reason to NULL when updating an existing tx

* add more log details when adding tx to AddrQueue

* fix query to add tx to the pool. Fix lint errors

* change failed_reason for tx discarded due duplicate nonce

* Only return a tx from the pool if tx is in pending status (#2273)

* Return a tx from the pool only if it is

* fix TestGetTransactionByHash

---------

Co-authored-by: agnusmor <agnusmor@gmail.com>

* fix documentation with  config file

* improve: adding check to skip appending effectivePercentage if current forkId is under 5.

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* Fiex effectiveGasprice unsigned txs with forkId lower than 5 (#2278)

* feat: adding functionality to stop sequencer on specific batch num from config param.

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* patch: adding print for X-Real-IP in JSON-RPC

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* Fix checkIfSynced (#2289)

* [Rehashing] Check logs order and fix blockhash and blockNumber in the log conversion (#2280)

* fix and check order

* linter

* flushID synchronizer (#2287)

* FlushID in synchronizer

* linter

* fix logs

* commnets

* executor error refactor (#2299)

* handle invalid rlp ROM error (#2297)

* add maxL2GasPrice (#2294)

* add maxL2GasPrice

* fix

* fix

* add test

* document parameter

* update description

* Error refactor (#2302)

* error refactor

* refactor

* Fix replaced tx as failed when duplicated nonce (#2308)

* Fix UpdateTxStatus for replacedTx

* Fix adding tx with same nonce on AddrQueue

* log reprocess need (#2309)

* log reprocess need

* Update finalizer.go

* Feature/2300 synchronizer detect if executor restart (#2306)

* detect if executor restarts and stop synchonizer

* Update prover images (#2311)

* update prover image

* update prover images

* change executor param

* Update testnet.prover.config.json

* Update test.permissionless.prover.config.json

* Update test.prover.config.json

* Update public.prover.config.json

* prover params

* prover params

* prover params

* update prover images

* add doc, and fix dockers to be able to use snap/restore feature (#2315)

* add doc, and fix dockers to be able to use snap/restore feature

* add doc for snap/restore feature

---------

Co-authored-by: Toni Ramírez <toni@polygon.technology>

* Update docker-compose.yml

* Update docker-compose.yml

* do not add tx to the pool in case err != nil

* do not add tx into the pool if a fatal error in the executor happens during pre execution

* fix dbMultiWriteSinglePosition config value

* workarround for the error error closing batch

* workarround for the error error closing batch

* workarround for the error error closing batch

* workaround for the error of closing batch, another case

* `Worker`'s `AddTxTracker` Bug Fix (#2343)

* bugfix: Resolve  Function Bug in Worker Module

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* improve: improving the wait for pending txs to be for only the txs for the current address.

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

---------

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* rename config files (#2349)

* fix closing batch + logs (#2348)

* fix closing batch + logs

* fix

* log description

* typo errors

* fix error: failed to store transactions for batch due to duplicate key

* test

* typo

* Update README.md

* Update release.yml

* bugfix: fixing place where we need to increment the wg per address for pending txs

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* Store batchL2Data when the batch is opened (#2358)

* add GasPriceMarginFactor and MaxGasPrice to eth-tx-manager (#2360)

* add GasPriceMarginFactor and MaxGasPrice to eth-tx-manager

* add logs, fix config

* update config file documentation

---------

Co-authored-by: joanestebanr <129153821+joanestebanr@users.noreply.github.com>

* bugfix: attaching missing TxTracker.From to pending txs to store for forced batches. (#2365)

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* Update README.md

* improve: adding logs (#2373)

* improve: adding logs

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* adding more logs

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* adding more logs #2

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

---------

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* bugfix: fixing finalizer's  handling. (#2375)

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* Update README.md

* change hashdb go package

* new hashdb interface

* aggregator pb refactor

* new prover image

* change prover config

* update prover image

* update to latest proto and prover image

* Refactor nonce calculation for addQueue (#2382)

* refactor nonce

* fix

* fix

* fix script

* check to avoid data inconsistencies (#2387)

* check to avoid data inconsistencies

* check batchL2Data

* names in the logs

* Refactor: avoid delete addrQueue if it has pending txs to store (#2391)

* refactor delete addrQueue only if not pending txs to store

* fix finalizer test

* fix olsStateRoot in handleForcedTxsProcessResp

* Update sequencer/addrqueue.go

Co-authored-by: Alonso Rodriguez <ARR552@users.noreply.github.com>

---------

Co-authored-by: Toni Ramírez <58293609+ToniRamirezM@users.noreply.github.com>
Co-authored-by: Alonso Rodriguez <ARR552@users.noreply.github.com>

* Sort txs in worker by gasPrice (remove efficiency sort) (#2392)

* Sort txs in worker by GasPrice (remove efficiency sort)

* update config docs

---------

Co-authored-by: Toni Ramírez <toni@polygon.technology>

* use useMainExecGenerated (#2393)

* Fix store forced batch tx (#2394)

* l2coinbase (#2400)

* l2coinbase

* add default config

* add support  config fields that are common.Address

* docs

* prover image

---------

Co-authored-by: joanestebanr <129153821+joanestebanr@users.noreply.github.com>

* Check flushID != 0 (#2406)

* Show tx.GasPrice in the worker logs (instead of tx.Cost) (#2416)

* Check flushID != 0 in Sequencer (#2415)

* check flushid != 0 in sequencer

* Use f.halt instead of log.Fatal to report that flushid is 0

Co-authored-by: Toni Ramírez <58293609+ToniRamirezM@users.noreply.github.com>

* fix lint

---------

Co-authored-by: Toni Ramírez <58293609+ToniRamirezM@users.noreply.github.com>

* update config params for Prover v2.1.0 (#2418)

* cherry-pick #2385 and #2396 from develop into v0.2.6 (#2412)

* fix http request instance null for websocket requests (#2385)

* fix ws subscribe to get filtered log notifications (#2396)

* new block endpoints and improvements to batch endpoint (#2411)

* Add forced batches tx to addrQueue (#2398)

* add forced batches tx to addrQueue

* fix test

* fix test

* fix test

* fix test

* fix test

* fix test

* fix test

* fix test

* fix test

* refactor

* fix test

* fix test

* fix test

* fix test

* fix test

* fixes

* fixes

* fixes

* fixes

* fixes

* fixes

* fixes

* fixes

* fixes

* fixes

* fixes

* fixe hash and from

* fixe hash and from

* fixe hash and from

* fixe hash and from

* fix test

* fix test

* fix test

* fix test

* fix test

* fix test

* fix test

* improve tests

* improve tests

* improve tests

* improve tests

* improve tests

* refactor

* refactor

* improve logs

* bugifx: adding missing tx.BreakEvenGasPrice nil check

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* Sync halt (#2428)

* cherry-pick #2245 and #2424 from develop into v0.2.6 (#2447)

* fix safe and finalized l2 block to consider l1 safe and finalized blocks respectively (#2245)

* fix and add tests for safe and finalized l2 blocks (#2424)

* New executor errors refactor (#2438)

* wip

* new errors

* retry on executor db error

* new prover images

* fix comment

* update hasdh proto and prover images

* handle excutor db error

* update test

* update test

* update test

* update test

* refactor error check in unsigned tx

* Reprocess full batch in parallel (sanity check) (#2425)

* reprocess full batch in parallel (sanity check)

* update doc

* update reprocessFullBatch logs

* Speed up deleting batches from stateDB creating an index for state.receipt.block_num (#2457)

* receipt deletion index

* receipt deletion index

* Feature/#2429 fork id improvement (#2450)

* db table + tests

* GetForks func modified to get them by range

* Sync forkID

* forkIDIntervals and forkID genesis

* linter

* docs

* Avoid resetForkID in trustedNode

* fix test group 9

* suggestions

* doc and mocks

* fix check storedFlushID (#2458)

* remove stored flush id 0 (#2459)

* Feature/#2403 snap (#2404)

* Path snapshot command

* restore

* readme

* options used by dbeaver

* #2429_forkID_improvement: #2429_forkID_improvement:

* fix

* fix postgres version to v15

* fix permissionless init script

* bugfix: removing measuring of metrics from async batch reprocessing f… (#2461)

* bugfix: removing measuring of metrics from async batch reprocessing for executor.

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* fixing unit tests

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

---------

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>

* update prover images (#2473)

* Update production-setup.md

* update doc

* fix jsonrpc tests

* fixes state.db

* update doc again

* remove obsolete config

* docs one more time...

---------

Signed-off-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>
Co-authored-by: joanestebanr <zodiac.ng@gmail.com>
Co-authored-by: Alonso Rodriguez <ARR552@users.noreply.github.com>
Co-authored-by: tclemos <thiago@polygon.technology>
Co-authored-by: tclemos <thiago@iden3.com>
Co-authored-by: agnusmor <agnusmor@gmail.com>
Co-authored-by: agnusmor <100322135+agnusmor@users.noreply.github.com>
Co-authored-by: Thiago Coimbra Lemos <tclemos@users.noreply.github.com>
Co-authored-by: joanestebanr <129153821+joanestebanr@users.noreply.github.com>
Co-authored-by: Nikolay Nedkov <nikolai_nedkov@yahoo.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants