tests:f3: Basic itests coverage for F3 #12204
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Alfonso de la Rocha <adlrocha@tutamail.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
…to f3-itests Signed-off-by: Alfonso de la Rocha <adlrocha@tutamail.com>
adlrocha
commented
Jul 10, 2024
| const F3BootstrapEpoch abi.ChainEpoch = -1 | ||
| // TODO: We need F3Enabled to be public because without it there is no way to | ||
| // allow f3 pubsub topics without a lot of refactoring. | ||
| var F3Enabled = false |
There was a problem hiding this comment.
Unfortunately, this one needs to be public so it can be enabled at will in itests. Without them, pubsub F3 topics won't be allowed. For the rest we don't need it because we manually build the node F3-enabled through DI.
There may be a better way to do this, suggestions welcome.
adlrocha
commented
Jul 10, 2024
| var F3Enabled = false | ||
| var ManifestServerID = "12D3KooWENMwUF9YxvQxar7uBWJtZkA6amvK4xWmKXfSiHUo2Qq7" | ||
| var F3BootstrapEpoch abi.ChainEpoch = -1 | ||
| var F3Finality = Finality |
There was a problem hiding this comment.
Same here, this is convenient so we can change the finality considered for F3 in itests.
Signed-off-by: Alfonso de la Rocha <adlrocha@tutamail.com>
rvagg
reviewed
Jul 15, 2024
| one, two TestMiner | ||
| ) | ||
| minerOpts := append(nopts, ConstructorOpts( | ||
| node.Override(node.F3Participation, modules.F3Participation), |
There was a problem hiding this comment.
could you just add a new option to enable f3 participation to the ensemble rather than an entire new constructor just to add this?
There was a problem hiding this comment.
oh, you have one below, F3Enabled, so why is this needed?
rvagg
reviewed
Jul 15, 2024
| @@ -215,6 +218,19 @@ func MutateSealingConfig(mut func(sc *config.SealingConfig)) NodeOpt { | |||
| }))) | |||
| } | |||
|
|
|||
| func F3Enabled(bootstrapEpoch abi.ChainEpoch, blockDelay time.Duration, finality abi.ChainEpoch) NodeOpt { | |||
There was a problem hiding this comment.
Suggested change
| func F3Enabled(bootstrapEpoch abi.ChainEpoch, blockDelay time.Duration, finality abi.ChainEpoch) NodeOpt { | |
| func WithF3Enabled(bootstrapEpoch abi.ChainEpoch, blockDelay time.Duration, finality abi.ChainEpoch) NodeOpt { |
I know there's a mixed pattern in here but it'd be nice to default to With options
Stebalien
added a commit
that referenced
this pull request
Aug 14, 2024
This way, we don't have to rely on a global variable to determine if F3 is enabled, which will make testing easier. Pre-requisite for #12204
Stebalien
added a commit
that referenced
this pull request
Aug 14, 2024
This way, we don't have to rely on a global variable to determine if F3 is enabled, which will make testing easier. Pre-requisite for #12204
8 tasks
Stebalien
added a commit
that referenced
this pull request
Aug 14, 2024
This way, we don't have to rely on a global variable to determine if F3 is enabled, which will make testing easier. Pre-requisite for #12204
Stebalien
added a commit
that referenced
this pull request
Aug 14, 2024
This way, we don't have to rely on a global variable to determine if F3 is enabled, which will make testing easier. Pre-requisite for #12204
Stebalien
added a commit
that referenced
this pull request
Aug 14, 2024
This way, we don't have to rely on a global variable to determine if F3 is enabled, which will make testing easier. Pre-requisite for #12204
Stebalien
added a commit
that referenced
this pull request
Aug 14, 2024
This way, we don't have to rely on a global variable to determine if F3 is enabled, which will make testing easier. Pre-requisite for #12204
rjan90
added this to the Milestone 2: F3 Harderning and Mainnet Deployment Readiness milestone
8 tasks
|
Replacing with #12486. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issues
Depends on #12196
Proposed Changes
This PR covers some basic itests for F3, mainly:
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, mempool, multisig, networking, paych, proving, sealing, wallet, deps[skip changelog]to the PR titleskip-changelogto the PR