Refactor validation: introduce the Consensus trait #1656

aakoshh · 2022-07-14T00:27:27Z

Summary of changes
Changes introduced in this pull request:

Added a chain_sync::consensus::Consensus trait which currently has a single validate_block method for consensus specific rules. It also has an associated error type.
Added a blockchain/fil_consensus crate with FilecoinConsensus implementing the Consensus trait. Moved the errors and validations specific to PoSt, tickets, beacons, DRAND, etc. into this crate. FilecoinConsensus also captures the generic beacon type and the ProofValidator.
The structs in tipset_syncer and ChainMuxer get a generic consensus: C where C: Consensus argument so they can delegate the custom validations to it.
They keep the common validation which is related to the messages and the ledger (gas, account sequences, state, fees).
Because the ProofValidator was moved into FilecoinConsensus I was able to remove it from all but one method in the RPC and StateManager where a block is assembled.

Other information and links
This is laying some groundworks towards implementing Hierarchical Consensus

In the future we want to expand this interface to be able to generate blocks on a schedule, adhering to some hitherto unknown consensus protocol.

…rError

CLAassistant · 2022-07-14T00:27:31Z

All committers have signed the CLA.

codecov · 2022-07-14T06:13:38Z

Codecov Report

Merging #1656 (ac5338b) into main (a233e72) will decrease coverage by 0.11%.
The diff coverage is 0.17%.

@@            Coverage Diff             @@
##             main    #1656      +/-   ##
==========================================
- Coverage   28.68%   28.57%   -0.12%     
==========================================
  Files         189      192       +3     
  Lines       20863    20934      +71     
==========================================
- Hits         5985     5982       -3     
- Misses      14878    14952      +74

Impacted Files	Coverage Δ
blockchain/chain/src/store/chain_store.rs	`34.23% <ø> (ø)`
blockchain/chain_sync/src/chain_muxer.rs	`5.57% <0.00%> (+<0.01%)`	⬆️
blockchain/chain_sync/src/consensus.rs	`0.00% <0.00%> (ø)`
blockchain/chain_sync/src/lib.rs	`100.00% <ø> (ø)`
blockchain/chain_sync/src/tipset_syncer.rs	`7.40% <0.00%> (+1.23%)`	⬆️
blockchain/fil_consensus/src/validation.rs	`0.00% <0.00%> (ø)`
blockchain/message_pool/src/msgpool/msg_pool.rs	`65.34% <0.00%> (-0.07%)`	⬇️
blockchain/message_pool/src/msgpool/provider.rs	`23.52% <0.00%> (+1.30%)`	⬆️
...ockchain/message_pool/src/msgpool/test_provider.rs	`76.79% <0.00%> (+1.66%)`	⬆️
blockchain/state_manager/src/lib.rs	`2.77% <0.00%> (+0.06%)`	⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a233e72...ac5338b. Read the comment docs.

lemmih · 2022-07-14T08:11:50Z

Looks good. Mind adding license headers to the two new files? It can be done by running make license in the repository root.

aakoshh · 2022-07-14T10:41:21Z

Thanks @lemmih I added the license and resolved the clippy issues, also signed the CLA.

LesnyRumcajs

Really nice PR, a lot of documentation, thanks! Control question, did you check if it syncs on mainnet and calibnet?

blockchain/chain_sync/src/tipset_syncer.rs

aakoshh · 2022-07-14T15:52:29Z

did you check if it syncs on mainnet and calibnet?

No. I see in the readme how to start it with calibnet, but not where I should get a snapshot.car file from.

How do you run these tests?

LesnyRumcajs · 2022-07-14T16:05:12Z

did you check if it syncs on mainnet and calibnet?

No. I see in the readme how to start it with calibnet, but not where I should get a snapshot.car file from.

How do you run these tests?

Right now those snapshots are generated with Lotus, there is a PR in progress to get out of this dependency. For now you can download a recent one here

To run calibnet on NV16 this PR needs to be merged first. Mainnet should be fine though.

Basically for mainnet:

 target/release/forest --target-peer-count 50 --encrypt-keystore false --import-snapshot ~/prj/snapshots/minimal_finality_stateroots_1974240_2022-07-11_10-00-00.car

And observe it if syncs for some time.

Definitely, a piece of documentation (and automated testing) we should work on!

aakoshh · 2022-07-14T19:25:40Z

I found a link https://fil-chain-snapshots-fallback.s3.amazonaws.com/mainnet/minimal_finality_stateroots_latest.car to the mainnet snapshot but it's over 70Gb. I'm not going to download that, but tried the sync without it and it seems to work all right:

❯ target/release/forest --target-peer-count 50 --encrypt-keystore false                                                                                                                                          ─╯
 2022-07-14T19:24:36.043Z WARN  forest::cli > No configurations found, using defaults.
 2022-07-14T19:24:36.044Z INFO  forest::daemon > Starting Forest daemon, version v0.2.2/unstable/65e3dcff
 2022-07-14T19:24:36.044Z INFO  forest_libp2p::service > Recovered libp2p keypair from "/home/aakoshh/.local/share/forest/libp2p/keypair"
 2022-07-14T19:24:36.044Z WARN  forest::daemon         > Warning: Keystore encryption disabled!
 2022-07-14T19:24:36.044Z WARN  key_management::keystore > Keystore does not exist, initializing new keystore at: "/home/aakoshh/.local/share/forest/keystore.json"
 2022-07-14T19:24:36.044Z INFO  utils                    > Permissions set to 0600 on File { fd: 6, path: "/home/aakoshh/.local/share/forest/keystore.json", read: false, write: true }
 2022-07-14T19:24:36.044Z INFO  forest::daemon           > Admin token: ...
 2022-07-14T19:24:36.044Z INFO  metrics                  > Prometheus server started at 127.0.0.1:6116
 2022-07-14T19:24:36.090Z WARN  chain::store::chain_store > No previous chain state found
 2022-07-14T19:24:36.110Z INFO  genesis                   > Initialized genesis: BlockHeader: Cid(bafy2bzacecnamqgqmifpluoeldx7zzglxcljo6oja4vrmtj7432rphldpdmm2)
 2022-07-14T19:24:36.111Z INFO  forest::daemon            > Using network :: cannot get name
 2022-07-14T19:24:36.563Z INFO  forest::daemon            > JSON-RPC endpoint started at 127.0.0.1:1234
 2022-07-14T19:24:36.563Z INFO  rpc                       > Ready for RPC connections
 2022-07-14T19:24:37.974Z INFO  chain_sync::chain_muxer   > Local node is behind the network, starting BOOTSTRAP from LOCAL_HEAD = 0 -> NETWORK_HEAD = 1984009
Downloading headers 3614 / 1984009 [>-------------------------------------------------------------------------------------------------------------------------------------------------------] 0.18 % 147.05/s 224m ^C 2022-07-14T19:25:02.716Z WARN  forest::cli               > Got interrupt, shutting down...
 2022-07-14T19:25:02.717Z INFO  utils                     > Permissions set to 0600 on File { fd: 24, path: "/home/aakoshh/.local/share/forest/keystore.json", read: false, write: true }
 2022-07-14T19:25:02.746Z INFO  forest::daemon            > Forest finish shutdown.

I guess it's now downloading all the headers in reverse, which means I should see some blocks being imported in ~4 hours ⏳

aakoshh · 2022-07-14T22:18:44Z

@LesnyRumcajs the sync with the mainnet ran into some issue with 90 minutes left on the clock, it found no peers to request headers from.

So I created a separate branch from this where I merged the PR you mentioned and tried syncing with calibnet instead, using the snapshot you linked to. That seems to have worked:

 ./target/release/forest --chain calibnet --import-snapshot ../calibnet-2022-07-01.car --target-peer-count 50 --encrypt-keystore false                                                                          ─╯
 2022-07-14T22:04:08.309Z WARN  forest::cli > No configurations found, using defaults.
 2022-07-14T22:04:08.309Z INFO  forest::daemon > Starting Forest daemon, version v0.2.2/unstable/e6206af3
 2022-07-14T22:04:08.309Z INFO  forest_libp2p::service > Recovered libp2p keypair from "/home/aakoshh/.local/share/forest/libp2p/keypair"
 2022-07-14T22:04:08.309Z WARN  forest::daemon         > Warning: Keystore encryption disabled!
 2022-07-14T22:04:08.310Z INFO  forest::daemon         > Admin token: eyJ0eXAiOiJKV1QiLCJhbGciOi...
 2022-07-14T22:04:08.310Z INFO  metrics                > Prometheus server started at 127.0.0.1:6116
 2022-07-14T22:04:08.376Z INFO  genesis                > Initialized genesis: BlockHeader: Cid(bafy2bzacecz3trtejxtzix4f4eebs7dekm6snfsmvffiqz2rfx7iwgsgtieq4)
 2022-07-14T22:04:08.376Z INFO  forest::daemon         > Using network :: cannot get name
 2022-07-14T22:04:08.376Z INFO  genesis                > Importing chain from snapshot
 2022-07-14T22:04:08.376Z INFO  genesis                > Reading file...
Scanning blockchain 1082530 / 1087290 [=====================================================================================================================================================] 99.56 % 8530.18/s 1s  
2022-07-14T22:06:45.956Z INFO  genesis                > Accepting [Cid(bafy2bzacea7c2aqujxac37aeizzg5hr2eudwumppgcek5qvxmrkp3gjv7jlm6), Cid(bafy2bzacebl2enajghfcgzvgxf66syddba4adfsexz74jca3ygxbdb2jae7bg), Cid(bafy2bzacedjwvigkrlyzvn6qvxzygw7u2qqxqywc7i7wnwqkcxfk7rf75lovw), Cid(bafy2bzacecrx6cmjyobmuaexgascuqh3ad4kxh3ff2bdxjecmusvwvs22w65w), Cid(bafy2bzacebrv2rkkqvqxuoeou2a55xwhjqsyyujic54sfsfzxuqjo2tirfk4a), Cid(bafy2bzacedd76nyajhq2bphw3aztdmtlwtrrawcvvkrcebdq6lreabqig3wyy)] as new head.
 2022-07-14T22:06:46.433Z INFO  forest::daemon         > JSON-RPC endpoint started at 127.0.0.1:1234
 2022-07-14T22:06:46.433Z INFO  rpc                    > Ready for RPC connections
 2022-07-14T22:06:48.521Z INFO  chain_sync::chain_muxer > Local node is behind the network, starting BOOTSTRAP from LOCAL_HEAD = 1087290 -> NETWORK_HEAD = 1125853
Downloading headers 38563 / 38563 [=============================================================================================================================================================] 100.00 % 97.03/s  
2022-07-14T22:13:27.051Z INFO  chain_sync::tipset_syncer > Validating tipset: EPOCH = 1087290
 2022-07-14T22:13:30.707Z INFO  chain_sync::tipset_syncer > Validating tipset: EPOCH = 1087291
 2022-07-14T22:13:30.764Z INFO  chain_sync::tipset_syncer > Validating tipset: EPOCH = 1087292
 2022-07-14T22:13:31.161Z INFO  chain_sync::tipset_syncer > Validating tipset: EPOCH = 1087293
...

LesnyRumcajs · 2022-07-15T09:08:25Z

@aakoshh You can't really sync without the snapshots as Forest doesn't have any migrations at the moment - only pure NV16. Even with those in place, it would take several weeks to sync. But it's a good point for exiting early instead of raising hopes and expectations!

Glad it works, thanks for checking it out!

blockchain/chain_sync/src/consensus.rs

blockchain/chain_sync/src/tipset_syncer.rs

elmattic

lgtm, nice refactor!

aakoshh added 9 commits July 12, 2022 20:28

Add a few docstrings to sync and validation methods.

4570e78

Move Filecoin consensus specific errors out of TipsetRangeSyncrhonize…

eba364e

…rError

Move Filecoin consensus into its own crate.

edd76a4

Temporarily comment out validation.

c010793

Thread through a consensus instance. Remove unused generics.

6968641

Remove ProofVerifier from StateManager, VM and MessagePool

300cf7f

Remove ProofVerifier from RPC where not needed.

e52914e

Move Filecoin specific validation rules to the consensus crate.

326b26a

Merge remote-tracking branch 'origin/main' into refactor-validation

be772ac

aakoshh requested review from connormullett, elmattic, lemmih, LesnyRumcajs, jdjaustin and tyshko5 as code owners July 14, 2022 00:27

aakoshh changed the title ~~Refactor validation: add the Consensus trait~~ Refactor validation: introduce the Consensus trait Jul 14, 2022

aakoshh added 3 commits July 14, 2022 10:03

Add license.

0d52374

Fix docstrings.

ef59914

Fix clippy warnings.

a09c274

LesnyRumcajs reviewed Jul 14, 2022

View reviewed changes

blockchain/chain_sync/src/tipset_syncer.rs Show resolved Hide resolved

blockchain/chain_sync/src/tipset_syncer.rs Outdated Show resolved Hide resolved

Error CSV function.

e616412

aakoshh added 3 commits July 14, 2022 19:51

Merge remote-tracking branch 'origin/main' into refactor-validation

536e155

Collect msgs with expression.

e1a3705

Merge remote-tracking branch 'origin/main' into refactor-validation

65e3dcf

LesnyRumcajs approved these changes Jul 15, 2022

View reviewed changes

LesnyRumcajs mentioned this pull request Jul 15, 2022

Early exit Forest if no snapshot is provided and the DB is empty #1669

Closed

elmattic reviewed Jul 15, 2022

View reviewed changes

blockchain/chain_sync/src/consensus.rs Outdated Show resolved Hide resolved

elmattic reviewed Jul 15, 2022

View reviewed changes

blockchain/chain_sync/src/tipset_syncer.rs Outdated Show resolved Hide resolved

elmattic reviewed Jul 15, 2022

View reviewed changes

blockchain/chain_sync/src/tipset_syncer.rs Outdated Show resolved Hide resolved

elmattic requested changes Jul 15, 2022

View reviewed changes

aakoshh added 3 commits July 15, 2022 12:17

Fix typo.

8da6fbb

Remove unused errors.

d7f0c3f

Merge remote-tracking branch 'origin/main' into refactor-validation

ac5338b

aakoshh requested a review from elmattic July 15, 2022 11:32

elmattic approved these changes Jul 15, 2022

View reviewed changes

elmattic merged commit f73daa6 into ChainSafe:main Jul 15, 2022

aakoshh deleted the refactor-validation branch July 15, 2022 13:23

This was referenced Jul 15, 2022

Factor out consensus specific validation aakoshh/forest#1

Closed

Move chain::weight into Consensus #1680

Merged

🎯 | B3.2-M1: Consensus interface and dummy consensus in Forest consensus-shipyard/consensuslab#127

Closed

adlrocha mentioned this pull request Jul 19, 2022

🚧 | B3: Turn B1 into production-ready system for core actors and FVM consensus-shipyard/consensuslab#6

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor validation: introduce the Consensus trait #1656

Refactor validation: introduce the Consensus trait #1656

aakoshh commented Jul 14, 2022 •

edited

Loading

CLAassistant commented Jul 14, 2022 •

edited

Loading

codecov bot commented Jul 14, 2022 •

edited

Loading

lemmih commented Jul 14, 2022

aakoshh commented Jul 14, 2022

LesnyRumcajs left a comment

aakoshh commented Jul 14, 2022

LesnyRumcajs commented Jul 14, 2022 •

edited

Loading

aakoshh commented Jul 14, 2022 •

edited

Loading

aakoshh commented Jul 14, 2022 •

edited

Loading

LesnyRumcajs commented Jul 15, 2022

elmattic left a comment

Refactor validation: introduce the Consensus trait #1656

Refactor validation: introduce the Consensus trait #1656

Conversation

aakoshh commented Jul 14, 2022 • edited Loading

CLAassistant commented Jul 14, 2022 • edited Loading

codecov bot commented Jul 14, 2022 • edited Loading

Codecov Report

lemmih commented Jul 14, 2022

aakoshh commented Jul 14, 2022

LesnyRumcajs left a comment

Choose a reason for hiding this comment

aakoshh commented Jul 14, 2022

LesnyRumcajs commented Jul 14, 2022 • edited Loading

aakoshh commented Jul 14, 2022 • edited Loading

aakoshh commented Jul 14, 2022 • edited Loading

LesnyRumcajs commented Jul 15, 2022

elmattic left a comment

Choose a reason for hiding this comment

aakoshh commented Jul 14, 2022 •

edited

Loading

CLAassistant commented Jul 14, 2022 •

edited

Loading

codecov bot commented Jul 14, 2022 •

edited

Loading

LesnyRumcajs commented Jul 14, 2022 •

edited

Loading

aakoshh commented Jul 14, 2022 •

edited

Loading

aakoshh commented Jul 14, 2022 •

edited

Loading