adapt integration tests for consensus v2 #6685

ssd04 · 2024-12-18T15:49:30Z

Reasoning behind the pull request

Adapted integration test to use a full node with consensus and processing components
Use full node for testing with invalid signers flow
Added more tests for basic sync process

Testing procedure

TBD

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

was the PR targeted to the correct branch?
if this is a larger feature that probably needs more than one PR, is there a feat branch created?
if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

…-tests

…-integration-tests

…-tests

…tegration-tests

…-tests

sstanculeanu · 2025-02-04T15:35:07Z

process/block/baseProcess.go


 func (bp *baseProcessor) hasMissingProof(headerInfo *hdrInfo, hdrHash string) bool {
-	isFlagEnabledForHeader := bp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, headerInfo.hdr.GetEpoch())
+	isFlagEnabledForHeader := common.ShouldBlockHavePrevProof(headerInfo.hdr, bp.enableEpochsHandler, common.EquivalentMessagesFlag)


I think this should stay as before, as we check the proof for header, not the prev proof

yes, it should be the previous check proof for header not prev proof.

reverted + added check for genesis block (this is needed mainly for integration tests)

sstanculeanu · 2025-02-04T15:35:41Z

process/block/metablock.go


 func (mp *metaProcessor) checkProofsForShardData(header *block.MetaBlock) error {
-	if !mp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, header.Epoch) {
+	if !common.ShouldBlockHavePrevProof(header, mp.enableEpochsHandler, common.EquivalentMessagesFlag) {


sstanculeanu · 2025-02-04T15:36:07Z

process/block/metablock.go


 		isBlockAfterEquivalentMessagesFlag := !check.IfNil(headerInfo.hdr) &&
-			mp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, headerInfo.hdr.GetEpoch())
+			mp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, headerInfo.hdr.GetEpoch()) && headerInfo.hdr.GetNonce() > 1


is this needed?

added this for genesis block; it is helpful mainly for integration tests

we can discuss if we really want this approach

sstanculeanu · 2025-02-04T15:37:38Z

integrationTests/testConsensusNode.go

+	whiteListerVerifiedTxs, _ := interceptors.NewWhiteListDataVerifier(cacheVerified)
+
+	if tcn.ShardCoordinator.SelfId() == core.MetachainShardId {
+		metaInterceptorContainerFactoryArgs := interceptorscontainer.CommonInterceptorsContainerFactoryArgs{


L467-L498 is duplicated with L535-L567, can be extracted before this if

sstanculeanu · 2025-02-04T15:38:11Z

integrationTests/testFullNode.go

+	wasmConfig "github.com/multiversx/mx-chain-vm-go/config"
+)
+
+func CreateNodesWithTestFullNode(


missing mock comment

sstanculeanu · 2025-02-04T15:38:26Z

integrationTests/testFullNode.go

+	return nodes
+}
+
+type ArgsTestFullNode struct {


missing mock comment on all exported

sstanculeanu · 2025-02-04T15:39:50Z

integrationTests/testFullNode.go

+	whiteListerVerifiedTxs, _ := interceptors.NewWhiteListDataVerifier(cacheVerified)
+
+	if tcn.ShardCoordinator.SelfId() == core.MetachainShardId {
+		metaInterceptorContainerFactoryArgs := interceptorscontainer.CommonInterceptorsContainerFactoryArgs{


same note here, duplicated common args

AdoAdoAdo · 2025-01-31T13:30:40Z

integrationTests/consensus/consensusSigning_test.go

 	}

+	logger.ToggleLoggerName(true)
+	logger.SetLogLevel("*:TRACE,consensus:TRACE")


leaving it with trace?

AdoAdoAdo · 2025-01-31T13:52:35Z

integrationTests/consensus/consensusSigning_test.go

+		false,
 	)

-	for shardID, nodesList := range nodes {


are the nodes started somewehere else now?

yes, added a common function for this

AdoAdoAdo · 2025-01-31T14:01:48Z

integrationTests/testFullNode.go

+	}
+
+	var epochTrigger TestEpochStartTrigger
+	if tpn.ShardCoordinator.SelfId() == core.MetachainShardId {


can you extract each of the separate handling into a function?
e.g createEpochTrigger()
which creates it either for shard or for meta

AdoAdoAdo · 2025-01-31T14:02:40Z

integrationTests/testFullNode.go

+
+	tpn.BlockBlackListHandler = cache.NewTimeCache(TimeSpanForBadHeaders)
+
+	if tpn.ShardCoordinator.SelfId() != core.MetachainShardId {


here as well, the creation of the fork detector (for either shard or meta) could be done by a function/tpn method

AdoAdoAdo · 2025-01-31T14:11:31Z

integrationTests/testFullNode.go

+		tpn.EpochStartNotifier = notifier.NewEpochStartSubscriptionHandler()
+	}
+
+	if tpn.ShardCoordinator.SelfId() == core.MetachainShardId {


here as well we could have several methods defined

AdoAdoAdo · 2025-02-06T10:43:37Z

process/block/baseProcess.go


 func (bp *baseProcessor) hasMissingProof(headerInfo *hdrInfo, hdrHash string) bool {
-	isFlagEnabledForHeader := bp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, headerInfo.hdr.GetEpoch())
+	isFlagEnabledForHeader := common.ShouldBlockHavePrevProof(headerInfo.hdr, bp.enableEpochsHandler, common.EquivalentMessagesFlag)


yes, it should be the previous check proof for header not prev proof.

…-tests

adapt integration tests for consensus v2

6ecd517

ssd04 self-assigned this Dec 18, 2024

Base automatically changed from subround-endround-v2-fixes to feat/equivalent-messages December 19, 2024 08:11

AdoAdoAdo and others added 3 commits December 19, 2024 10:11

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

5c42932

…-tests

added separate single keys test with equivalent proofs

30f6c12

remove unused code

4a4f9b2

ssd04 changed the base branch from feat/equivalent-messages to equivalent-proofs-feat-stabilization January 15, 2025 13:25

Merge branch 'equivalent-proofs-feat-stabilization' into consensus-v2…

e9ec8bf

…-integration-tests

Base automatically changed from equivalent-proofs-feat-stabilization to feat/equivalent-messages January 20, 2025 11:41

ssd04 added 8 commits January 20, 2025 16:48

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

c7c2154

…-tests

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

27a42c9

…-tests

Merge branch 'equivalent-proofs-stabilization-2' into consensus-v2-in…

528d092

…tegration-tests

added full integration test - without equivalent proofs activation

6f29b71

integration tests meta chain fixes

e45434e

check for genesis block

bba0165

integration tests meta chain fixes

06ff80e

sync test fixes

22598c3

ssd04 changed the base branch from feat/equivalent-messages to equivalent-proofs-stabilization-2 January 28, 2025 16:22

ssd04 added 2 commits January 28, 2025 21:57

cleanup unused code

1451498

include genesis header check

9f6e666

Base automatically changed from equivalent-proofs-stabilization-2 to feat/equivalent-messages January 29, 2025 08:56

ssd04 added 9 commits January 31, 2025 12:33

update integration test with invalid signers

ad69364

added trace log messages

8b53a28

added sync test with more meta nodes

1887e15

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

c21a678

…-tests

fix linter issues

84b2255

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

2893cf2

…-tests

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

0ef4c06

…-tests

fix config after merge

92c1f0b

use common function for full node start

a32849b

fix linter issue

22edddc

ssd04 marked this pull request as ready for review January 31, 2025 13:57

sstanculeanu reviewed Feb 4, 2025

View reviewed changes

fixes after review

2716384

AdoAdoAdo reviewed Feb 6, 2025

View reviewed changes

ssd04 added 2 commits February 6, 2025 15:31

remove duplicated code

8a3ca7a

update check + additional check for genesis block

e5eb36a

sstanculeanu previously approved these changes Feb 6, 2025

View reviewed changes

sstanculeanu and others added 2 commits February 6, 2025 15:51

Merge branch 'feat/equivalent-messages' into consensus-v2-integration…

14e6645

…-tests

fix genesis time

ecbb55f

ssd04 dismissed sstanculeanu’s stale review via ecbb55f February 6, 2025 15:24

sstanculeanu approved these changes Feb 6, 2025

View reviewed changes

AdoAdoAdo approved these changes Feb 7, 2025

View reviewed changes

AdoAdoAdo merged commit 4a354c1 into feat/equivalent-messages Feb 7, 2025
4 checks passed

AdoAdoAdo deleted the consensus-v2-integration-tests branch February 7, 2025 08:08


		tpn.BlockBlackListHandler = cache.NewTimeCache(TimeSpanForBadHeaders)

		if tpn.ShardCoordinator.SelfId() != core.MetachainShardId {

adapt integration tests for consensus v2 #6685

adapt integration tests for consensus v2 #6685

Uh oh!

Conversation

ssd04 commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reasoning behind the pull request

Testing procedure

Pre-requisites

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ssd04 commented Dec 18, 2024 •

edited

Loading