[tmpnet] Minimize duration of tx acceptance for e2e testing #3685

maru-ava · 2025-01-30T00:02:35Z

Why this should be merged

Faster tests mean faster iteration

How this works

Enable configuration of min block delay for primary subnet with the addition of node flag --proposer-min-block-delay
Configure tmpnet to default the block delay to zero for all subnets
Reduce the wallet poll interval from 100ms to 10ms
Remove unnecessary sleep in virtuous test

How this was tested

CI against regression, manually verified decreased runtime

Need to be documented in RELEASES.md?

New node flag should be documented.

config/flags.go

qdm12

What's the overall time saved?
Reduce the wallet poll interval from 100ms to 10ms - just as a word of warning, getting close to millisecond in tests can be risky on CI running on shared/slow runners, and produce flaky tests, just because the runner is slow and things take milliseconds to execute... think Pentium 4 running CI 😄

qdm12 · 2025-01-30T10:37:24Z

tests/fixture/tmpnet/defaults.go

@@ -76,3 +78,11 @@ func DefaultChainConfigs() map[string]FlagsMap {
 		},
 	}
 }
+
+// A set of subnet configuration appropriate for testing.
+func DefaultSubnetConfig() FlagsMap {


since this is only used in this package, can we unexport it?

tmpnet is used by other repos - including subnet-evm and hypersdk - and the intention is for reuse.

maru-ava · 2025-01-30T16:30:55Z

What's the overall time saved?

I wasn't too concerned with total time saved. The goal was speeding up perceived execution speed when ~~running interactively~~ iterating. It is noticeably faster even without timing.

For a given transaction, a min block delay of 1s ensures that a set of sequentially executed transactions (as is commonly done in a test that uses a single wallet) take a minimum of 1s per transaction. In Stephen's example PR with a polling interval of 1ms, tx acceptance could approach that lower bound. I considered 1ms potentially problematic in the context of CI, though, so bumped the interval to 10ms. Going from a minimum of 1s to a minimum of 10ms is a pretty nice speedup!

Reduce the wallet poll interval from 100ms to 10ms - just as a word of warning, getting close to millisecond in tests can be risky on CI running on shared/slow runners, and produce flaky tests, just because the runner is slow and things take milliseconds to execute... think Pentium 4 running CI 😄

Sure, that's why I didn't go all the way down to 1ms. If its flakey, we'll adjust it.

StephenButtolph · 2025-02-03T02:35:01Z

config/config.go

@@ -1132,7 +1132,7 @@ func getDefaultSubnetConfig(v *viper.Viper) subnets.Config {
 	return subnets.Config{
 		ConsensusParameters:         getConsensusConfig(v),
 		ValidatorOnly:               false,
-		ProposerMinBlockDelay:       proposervm.DefaultMinBlockDelay,
+		ProposerMinBlockDelay:       v.GetDuration(ProposerMinBlockDelayKey),


This causes some weirdness in the defaults.

The default value of ProposerMinBlockDelay for all subnets is currently proposervm.DefaultMinBlockDelay.

With this change, the default value for subnets that do not specify a custom subnet config is still proposervm.DefaultMinBlockDelay.

With this change, the default value for subnets that do specify a custom subnet config is equal to --proposer-min-block-delay.

I think (2) and (3) should be unified.

I've updated loading of subnet config to set the default subnet config if one is not provided. PTAL

StephenButtolph · 2025-02-03T02:39:00Z

config/keys.go

@@ -208,4 +208,5 @@ const (
 	TracingExporterTypeKey                             = "tracing-exporter-type"
 	TracingHeadersKey                                  = "tracing-headers"
 	ProcessContextFileKey                              = "process-context-file"
+	ProposerMinBlockDelayKey                           = "proposer-min-block-delay"


Should we include this in RELEASES.md (and maybe in config.md as well?)

Added to config.md. How to add it to RELEASES?

StephenButtolph · 2025-02-03T02:39:43Z

config/keys.go

@@ -208,4 +208,5 @@ const (
 	TracingExporterTypeKey                             = "tracing-exporter-type"
 	TracingHeadersKey                                  = "tracing-headers"
 	ProcessContextFileKey                              = "process-context-file"
+	ProposerMinBlockDelayKey                           = "proposer-min-block-delay"


Also wondering, if this is only supposed to impact the primary network, if we should name it something like --primary-network-min-block-delay

Ok to rename it to --promposevm-min-block-delay or is --proposer-min-block-delay prefereable?

aaronbuchwald · 2025-02-24T19:13:21Z

config/config.go

@@ -1054,8 +1054,9 @@ func getSubnetConfigsFromFlags(v *viper.Viper, subnetIDs []ids.ID) (map[ids.ID]s

 	res := make(map[ids.ID]subnets.Config)
 	for _, subnetID := range subnetIDs {
+		config := getDefaultSubnetConfig(v)


I see, so here we guarantee that for each tracked subnetID we start with the default and override as provided.

aaronbuchwald · 2025-02-24T19:14:46Z

config/config.go

@@ -1090,11 +1091,14 @@ func getSubnetConfigsFromDir(v *viper.Viper, subnetIDs []ids.ID) (map[ids.ID]sub

 	// reads subnet config files from a path and given subnetIDs and returns a map.
 	for _, subnetID := range subnetIDs {
+		config := getDefaultSubnetConfig(v)


The comment added to chains/manager.go implies that we now require a default config is present for each subnetID that we're tracking. Do we need to guarantee that here as well? It seems that if subnetConfigPath is empty, then we'll exit before getting to this point and may not guarantee that?

It seems like this actually can't cause a problem because there are two behaviors:

use the default path where this will be non-empty and therefore not exit early

use a non-default path, where it will then check that the directory exists, which will cause an error if the subnetConfigPath is in fact an empty string

I think it's possible to supply an empty non-default path, so I think it will be necessary to ensure default values in that case.

aaronbuchwald · 2025-02-24T19:18:11Z

config/config_test.go

-		"subnet is not tracked": {
-			givenJSON: `{"Gmt4fuNsGJAd2PX86LBvycGaBpgCYKbuULdCLZs3SEs1Jx1LU":{"validatorOnly":true}}`,
-			testF: func(require *require.Assertions, given map[ids.ID]subnets.Config) {
-				require.Empty(given)
-			},
-			expectedErr: nil,
-		},


Should we keep this test as it's still relevant and should produce the same result as the new case default config used when no config provided ?

- Enable configuration of min block delay for primary subnet with the addition of node flag `--proposer-min-block-delay` - Configure tmpnet to default the block delay to zero for all subnets - Reduce the wallet poll interval from 100ms to 10ms - Remove unnecessary sleep in virtuous test

maru-ava added the testing label Jan 30, 2025

maru-ava self-assigned this Jan 30, 2025

maru-ava requested a review from StephenButtolph as a code owner January 30, 2025 00:02

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from d55ad5a to 021ba8f Compare January 30, 2025 00:14

maru-ava marked this pull request as draft January 30, 2025 00:51

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from 021ba8f to 6d51c62 Compare January 30, 2025 02:46

maru-ava marked this pull request as ready for review January 30, 2025 02:47

darioush reviewed Jan 30, 2025

View reviewed changes

config/flags.go Outdated Show resolved Hide resolved

qdm12 approved these changes Jan 30, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from 6d51c62 to a392bdb Compare January 30, 2025 18:33

darioush approved these changes Jan 30, 2025

View reviewed changes

StephenButtolph reviewed Feb 3, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from 5234800 to 579d425 Compare February 4, 2025 05:17

maru-ava requested a review from meaghanfitzgerald as a code owner February 4, 2025 05:17

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from a2ee6c0 to 0e05126 Compare February 17, 2025 12:57

aaronbuchwald reviewed Feb 24, 2025

View reviewed changes

maru-ava added 5 commits February 24, 2025 21:21

fixup: Respond to review feedback

168bd8b

fixup: lint

7541a71

fixup: Respond to reviewer feedback

aaa9a15

fixup: Fix unit tests

bc8bfee

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from 0e05126 to 63c9c73 Compare February 24, 2025 20:35

fixup: Respond to reviewer feedback

Loading
Loading status checks…

10da3ee

maru-ava force-pushed the tmpnet-minimize-tx-acceptance-duration branch from 63c9c73 to 10da3ee Compare February 24, 2025 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tmpnet] Minimize duration of tx acceptance for e2e testing #3685

[tmpnet] Minimize duration of tx acceptance for e2e testing #3685

maru-ava commented Jan 30, 2025

qdm12 left a comment

qdm12 Jan 30, 2025

maru-ava Jan 30, 2025

maru-ava commented Jan 30, 2025 •

edited

Loading

StephenButtolph Feb 3, 2025

maru-ava Feb 4, 2025

StephenButtolph Feb 3, 2025

maru-ava Feb 4, 2025

StephenButtolph Feb 3, 2025 •

edited

Loading

maru-ava Feb 4, 2025

aaronbuchwald Feb 24, 2025

aaronbuchwald Feb 24, 2025

aaronbuchwald Feb 24, 2025

maru-ava Feb 24, 2025

maru-ava Feb 24, 2025

aaronbuchwald Feb 24, 2025

maru-ava Feb 24, 2025

[tmpnet] Minimize duration of tx acceptance for e2e testing #3685

Are you sure you want to change the base?

[tmpnet] Minimize duration of tx acceptance for e2e testing #3685

Conversation

maru-ava commented Jan 30, 2025

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

qdm12 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maru-ava commented Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenButtolph Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maru-ava commented Jan 30, 2025 •

edited

Loading

StephenButtolph Feb 3, 2025 •

edited

Loading