Skip to content

Fixes MinBlockTime and adds artificial tier 1 topology flag - #438

Open
SirTyson wants to merge 9 commits into
mainfrom
min-block-time-methodology
Open

SirTyson wants to merge 9 commits into
mainfrom
min-block-time-methodology

Conversation

@SirTyson

@SirTyson SirTyson commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Stack: #437 → #438 → #439

Second of three stacked PRs. These are a variety of changes to MinBlockTimeTest , relevant to both v1 and v2. Commits are as follows:

  1. 45bf395 MinBlockTime: search whole-second close times only. Sub-second close times hit a known rounding issue (before ms closeTimes merge), so the search only tries whole seconds in [min, max], bounds included. I.e. default [4000, 5000] tries 4000, and then 5000 if 4000 fails. This will get reverted once ms close times lands and we can get intrasecond blocks.

  2. cc2c310 MinBlockTime: evaluate a single close time when min == max. Setting both bounds to the same value tests exactly that close time once, with no search. Useful for evaluating progress towards milestones at specific latencies.

  3. ed1ca52 Add --tier1-org-count. Flag to easily create a synthetic tier 1 topology with up to 40 organizations. This will create a geographically diverse simulated network with the specified number of tier-1 orgs, useful for benchmarking milestones.

  4. 4699d5a MinBlockTimeMixed: run MIXED_PREGEN load on every validator. Load used to come from one node per organization. Now every validator in the load-generating organizations submits an equal share, each from its own accounts. There's really no reason I can think of to limit the number of load generating nodes, and load gen is an expensive operation that can fail the test early.

  5. eaeb4c5 MinBlockTimeMixed: judge overlay-only candidates in mode. Hardens how overlay-only (soroban latency) candidates are judged. The inflated results we saw earlier came from v2 images whose overlay-only loadgen never counted TXs as included, so loadgen always failed and we only looked at ledger times (e.g. a "pass" at 2 second ledgers @ 3k TPS with only ~200 TPS actually included). Core fixed that in 9ec523e43f, and on master loadgen always counted inclusion, so loadgen's check isn't broken at the moment. However, we didn't realize this for some time, so it seems prudent to add some ssc based safeguards. This commit:

  • Treats a loadgen failure as a failed candidate instead of killing the mission, so the search continues. Loadgen only completes once every submitted TX has been included in a ledger; as a cross-check, at least 95% of the offered TXs must also appear in actual blocks.
  • Never turns apply mode back on after overlay-only mode. That handed nodes a large backlog of TXs to apply and made them lose sync, so we rely on the consistency and sync checks during the run instead, and restart the nodes between runs (after passes too) to clear the TX queue backlog.

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate issues remain in load sizing, input validation, and metric reporting.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity · 1 Low severity

Open (4)
What changed in this PR

Updates MinBlockTime benchmarking with configurable load sizing, whole-second search, expanded topology, per-validator load generation, and overlay-only judging.

Changes:

  • Adds CLI controls for transaction sizing, duration, byte allowances, and organization count.
  • Partitions mixed load and pre-generated transactions across validators.
  • Adds candidate inclusion checks, restart handling, tests, and documentation.
File Summary Final review findings
src/​FSLibrary/​StellarStatefulSets.fs Partitions validator load. No findings.
src/​FSLibrary/​StellarNetworkData.fs Adds synthetic Tier1 topology. No findings.
src/​FSLibrary/​StellarMissionContext.fs Stores new mission settings. No findings.
src/​FSLibrary/​StellarKubeSpecs.fs Supports per-validator pregeneration. No findings.
src/​FSLibrary/​StellarCoreCfg.fs Configures byte allowances. No findings.
src/​FSLibrary/​MinBlockTimeTest.fs Implements sizing, search, judging, and load behavior. Critical (1 vote): pregenerated transactions may be insufficient for the requested duration and rate. Moderate (3 votes): non-positive --txs-per-ledger values are not rejected. Moderate (2 votes): mixed mode omits E2E latency metrics. Nit (2 votes): result output incorrectly labels variable rates as fixed TPS.
src/​FSLibrary/​MaxTPSTest.fs Applies configurable Tier1 topology. No findings.
src/​FSLibrary.Tests/​Tests.fs Tests sizing, topology, partitioning, and search. Nit (1 vote): synthetic organization and region counts conflict with the documented contract.
src/​App/​Program.fs Exposes and wires new CLI options. No findings.
doc/​measuring-minimum-block-time.md Documents the methodology and options. No findings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated

{ baseLoadGen with
accounts = numAccounts
txs = totalT * context.loadDurationSec
Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
Comment on lines +715 to +726
match context.minBlockTimeTxsPerLedger with
| Some txsPerLedger when txsPerLedger > 0 ->
let nominalTotal = classicTxRateForLimits + sorobanTxRateForLimits

if nominalTotal <= 0 then
failwith "--txs-per-ledger needs a non-zero nominal TPS to derive the classic/soroban split"

let total = int ((int64 txsPerLedger * 1000L) / int64 targetMs)
let soroban = int ((int64 total * int64 sorobanTxRateForLimits) / int64 nominalTotal)
let classic = total - soroban
classic, soroban, total
| _ -> classicTxRateForLimits, sorobanTxRateForLimits, fixedTxRate
Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated

match bestPassing with
| Some t ->
LogInfo "Minimum sustainable block time: %d ms (fixed TPS %d, image %s)" t fixedTxRate context.image
Copilot AI review requested due to automatic review settings September 23, 2026 21:11

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate correctness issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity · 2 Medium severity · 1 Low severity

Open (5)

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from a8282ce to 99c4f9d Compare September 23, 2026 21:23
Copilot AI review requested due to automatic review settings September 23, 2026 21:23
@SirTyson
SirTyson force-pushed the harness-run-controls branch from 0c7c75d to 134b2e1 Compare September 23, 2026 21:23

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Unresolved critical load validation and multi-generator coordination issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 High severity · 2 Medium severity · 1 Low severity

Open (8)

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
Comment on lines +722 to +724
let total = int ((int64 txsPerLedger * 1000L) / int64 targetMs)
let soroban = int ((int64 total * int64 sorobanTxRateForLimits) / int64 nominalTotal)
let classic = total - soroban
Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated

{ baseLoadGen with
accounts = numAccounts
txs = totalT * context.loadDurationSec
Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
Copilot AI review requested due to automatic review settings September 23, 2026 21:44
@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from 99c4f9d to abdd1da Compare September 23, 2026 21:44
@SirTyson
SirTyson force-pushed the harness-run-controls branch from 134b2e1 to dd4baeb Compare September 23, 2026 21:44

Copilot AI left a comment

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.

@SirTyson
SirTyson force-pushed the harness-run-controls branch from dd4baeb to 99da71b Compare September 23, 2026 22:19
Copilot AI review requested due to automatic review settings September 23, 2026 22:19
@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from abdd1da to 853f1fa Compare September 23, 2026 22:19

Copilot AI left a comment

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.

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
@SirTyson
SirTyson force-pushed the harness-run-controls branch from 99da71b to 04160a6 Compare September 23, 2026 22:43
Copilot AI review requested due to automatic review settings September 23, 2026 22:43
@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from 853f1fa to df242c3 Compare September 23, 2026 22:43

Copilot AI left a comment

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.

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
Comment on lines +1001 to +1002
// Africa, Middle East). The base 10 use 13 distinct locations; with 19
// organizations there are 37, with all 40 there are 51.

Copilot AI left a comment

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.

@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from a3544e6 to eaeb4c5 Compare September 25, 2026 23:02
Copilot AI review requested due to automatic review settings September 25, 2026 23:02

Copilot AI left a comment

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.

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
Comment on lines +658 to +664
let mutable failureReason =
try
formation.RunMultiLoadgen activeLoadGenNodes loadGen
None
with e ->
LogError "Load generation FAILED at T=%dms: %s" targetMs e.Message
Some(sprintf "load generation failed: %s" e.Message)
@SirTyson
SirTyson force-pushed the harness-run-controls branch from 3cdf282 to 2026549 Compare September 30, 2026 08:29
Comment thread src/FSLibrary.Tests/Tests.fs Outdated
let candidates = [ 1000 .. 1000 .. 5000 ]

for threshold in candidates do
let evaluated = System.Collections.Generic.List<int>()

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.

nit: instead of using System.Collections.Generic.List, this could just use a mutable

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
// Reference measurements, 30 nodes at 500 SAC TPS (2026-07-27, image 3453):
// timeout=2000: externalize p75 2447ms, ledger-age p75 4592ms at T=3000 (+53%, fail)
// timeout=500: externalize p75 331ms, ledger-age p75 3028ms at T=3000 (+0.9%)
// The gap was stalled ballot rounds costing a full timeout before

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.

Given the other instabilities in the test, this reference measurement seems potentially a little suspect: e.g., why is the externalize p75 only higher than the timeout in one of the cases?

@SirTyson SirTyson Oct 1, 2026 •

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 think this has to do with bad snowballing behavior. Basically we can stall, such that no matter how long the timeout is, we have to wait for the next one. When that wait time is small (in the 500 ms case), we keep up with the load since the block is minimally stalled. With the longer timeout, we snowball, we have to wait so long such that later blocks become impacted more. I bevlieve this is why we see a much worse results in the 2000 case, due to the compounding factor. That being said, I didn't realize I didn't actually implement this behind the overlay-v2 flag so i'll move it to the follow up PR.


while hi - lo > searchThresholdMs do
if needsRecovery then restartCoreSetsOrWait ()
let needsRecovery = ref false

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.

nit: it's a little odd that this became a ref where it used to be mutable

Comment thread doc/measuring-minimum-block-time.md Outdated
* `--max-block-time-ms`: Binary search upper bound, in milliseconds. Defaults to `5000`, which is also the protocol's maximum allowed ledger target close time — setting this higher will cause the mission to fail at startup, since validators reject upgrades above the protocol cap. Must not be less than `--min-block-time-ms`; when the two are equal the mission evaluates exactly that close time once, with no search.
* `--num-pregenerated-txs`: Number of pre-generated signed classic transactions to create per loadgen node. `MinBlockTimeClassic` uses these on small networks (≤30 nodes) when it automatically switches classic payment load to `PayPregenerated`; `MinBlockTimeMixed` always uses them for the classic stream in its `MIXED_PREGEN_*` mode. Defaults to `2500000`
* `--pubnet-data`: Network topology to use. Defaults to a topology of tier 1 validators. See [Specifying network topologies](#specifying-network-topologies) for details on how to specify a custom topology.
* `--tier1-org-count`: Organizations (three validators each) in that default tier 1 topology, from `10` (the default) to `40`. Beyond 10, synthetic organizations are added in a fixed order (`x01`, `x02`, ...), spread over further cloud regions in North America, Europe, Asia, South America, Oceania, Africa and the Middle East; the simulated network delay between two validators grows with their distance, so larger counts also raise the network's latency floor.

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.

Can we unify this flag with --tier-1-orgs-to-add instead of adding a new flag that only applies in one of the circumstances?

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
checkLedgerAgeSLA ledgerAgePercentiles targetMs

if context.minBlockTimeMs >= context.maxBlockTimeMs then
// An explicit single-candidate target: --min-block-time-ms ==

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.

Is it intentional that the singleCandidate mode supports non-whole second times and times less than a second? Otherwise, it seems like the code can get cleaned up so that the single candidate mode just runs the binary search on one candidate.

Comment thread src/FSLibrary/StellarKubeSpecs.fs Outdated
// MinBlockTimeMixed's MIXED_PREGEN_* load runs on every validator rather than
// one node per load-generating core set, each with its own account slice (see
// PregenerationOptionsForPeer and StellarStatefulSets.LoadgenPeerIndices).
let LoadOnEveryValidator (ctx: MissionContext) (mode: LoadGenMode) : bool =

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.

nit: This doesn't seem like it logically belongs to StellarKubeSpecs. Maybe this should just be replaced with a check on ctx.pregenerateTxsPerValidator, and MinBlockTimeTest.fs can own the logic for determining when to set it.

Comment thread src/FSLibrary/StellarKubeSpecs.fs Outdated
Comment on lines +952 to +956
let test =
ShCmd [| ShWord.OfStr "test"
ShWord.Var CfgVal.peerNameEnvVarName
ShWord.OfStr "="
ShWord.OfStr name |]

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.

optional nit: this test might work better inside the getInitCommands (leads to a smaller increase in the generated script size)

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.

Claude says that the current change changes the behavior when initialization fails (previously, this would cause stellar-core not to start because of the chain of ShAnds).

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
let pregenerateTxs =
if isLoadGenNode cs then
let i = if isMixedPregenMode mode then j % partitionCount else j
if isLoadGenNode cs && (not (isMixedPregenMode mode) || isActiveLoadGenNode cs) then

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.

Could this just be isActiveLoadGenNode cs?

Comment thread src/FSLibrary/StellarStatefulSets.fs Outdated
// Divide aggregate rate, account space and duration across actual generators.
// In fixed-duration mode (LoadOnEveryValidator runs), derive each transaction
// budget from its assigned rate; equal transaction budgets would make
// remainder-rate peers finish early. Otherwise this is upstream's even split.

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.

nit: referring to "upstream" is odd for when this gets merged (iirc, this appears in at least one other place in this PR)

Comment thread src/FSLibrary/StellarStatefulSets.fs Outdated
// In fixed-duration mode (LoadOnEveryValidator runs), derive each transaction
// budget from its assigned rate; equal transaction budgets would make
// remainder-rate peers finish early. Otherwise this is upstream's even split.
let PartitionValidatorLoad (n: int) (fixedDuration: bool) (full: LoadGen) =

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.

nit: I'd like to see a return type on this

Base automatically changed from harness-run-controls to main October 1, 2026 17:43
Copilot AI lite review requested due to automatic review settings October 1, 2026 21:59
@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from eaeb4c5 to 03b6e2f Compare October 1, 2026 21:59

Copilot AI left a comment

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.

Comment thread src/FSLibrary/MinBlockTimeTest.fs Outdated
if List.isEmpty included || offered <= 0 then
None
else
let short = included |> List.filter (fun (_, txs) -> txs < float offered)
Copilot AI lite review requested due to automatic review settings October 1, 2026 22:42

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Four unresolved moderate findings must be addressed before approval.

Review effort: Lite
Findings: 4 High severity · 7 Medium severity · 2 Low severity

Open (13)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Missing promised --tier1-org-count option alias

src/​App/​Program.fs:504

The PR description names the new interface as --tier1-org-count, but this change only exposes/reuses --tier-1-orgs-to-add; there is no option or alias with the requested name. Users following the stated interface cannot invoke this feature, so add the promised alias or update the requirement consistently.

Comment on lines +292 to +296
let readAll () =
formation.NetworkCfg.PeersInSets(List.toArray coreSets)
|> List.map (fun peer -> async { return readLedgerTxsIncluded peer })
|> Async.Parallel
|> Async.RunSynchronously
Comment on lines +322 to +323
match included |> List.map snd |> List.distinct with
| [ txs ] when txs >= float offered ->
SirTyson and others added 9 commits October 1, 2026 15:58
Sub-second close times hit a known rounding issue in close-time
handling, so the search must never propose one. Search the whole
seconds in [--min-block-time-ms, --max-block-time-ms], bounds included,
for the smallest passing one. The default [4000, 5000] range evaluates
4000 and, only if that fails, 5000. A range with no whole second fails
with a clear error instead of evaluating nothing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
--min-block-time-ms == --max-block-time-ms == T evaluates exactly T
once, with no rounding and no search, through the same evaluation path.
Previously min == max was rejected. Fixed-target runs (e.g. T = 1000 ms)
no longer need a range and a binary search.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The synthetic Tier1 topology used by the MinBlockTime* and MaxTPS*
missions has 10 organizations. --tier1-org-count takes it up to 40 by
adding up to 30 synthetic organizations in a fixed order. At 40 they
use 38 locations the base topology does not (33 of them new), across
North America, Europe, Asia, South America, Oceania, Africa and the
Middle East. The simulated delay grows
with distance, so larger networks also get a realistic latency floor.
Without the flag the topology is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The overlay-only MIXED_PREGEN_* load ran on node 0 of each
load-generating organization. Run it on every validator of those
organizations instead, each with its own disjoint slice of the genesis
accounts and pre-generated transactions and an equal share of the
offered rate for the whole load window. Rate remainders go to the first
peers, and each peer's transaction budget follows its own rate so all
peers submit for the same duration. The run uses at most one generator
per requested TPS, counting every validator of an organization, so no
validator gets a zero share; only the organizations it uses get
pre-generated transactions.

Per-validator pre-generation is keyed on the mission's actual load
mode, so MinBlockTimeClassic and other missions keep one shared
pre-generation command. Other loadgen paths keep the existing split.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A loadgen failure now fails the candidate in every mode and the search
continues, as the doc already says, instead of aborting the mission.

Overlay-only candidates never apply their transactions, but loadgen
still completes only once every transaction its node submitted has been
included in a closed ledger: stellar-core counts inclusion in this mode
(the herder's self-delay timers on master; the self-count counters on
the Rust-overlay core since 9ec523e43f, 2026-07-31). A failure, such as
load left out of ledgers, therefore fails the candidate. Everything
else is judged while still in overlay-only mode: the close-time SLA,
the consistency and sync checks, and, as a cross-check from the
ledgers' own counts, at least 95% of the offered transactions reaching
ledgers (the ledger.transaction.count total over the window).
--measure-e2e-latency output is logged in this mode too.

Apply is not re-enabled after the window: turning it back on handed the
nodes whatever was left in their queues to apply at once, which pushed
them out of sync. The nodes are restarted before the next candidate,
after a pass as well as a failure. The SLA thresholds are logged per
candidate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With load on every validator, each pod's init commands ran inside an
`if test $POD = <name>; then { ...; }; fi` per validator. Within the
braces the commands were sequenced with `;` rather than `&&`, and the
`if` of a non-matching pod returns 0, so a failing new-db or
pregenerate-loadgen-txs no longer kept stellar-core from starting.

Only the pregeneration step differs per validator, so getInitCommands
now builds one init chain for the pod and branches just that step: an
if/elif per validator, with `else false` so a pod matching none fails.
The branch is part of the `&&` chain again, so any init failure stops
the pod before core runs, as for every other core set, and the script
holds the shared steps once instead of once per validator.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Instead of a separate --tier1-org-count that only applies to the
synthetic topology, --tier-1-orgs-to-add now means "tier 1 organizations
to add" in both cases: with --pubnet-data it scales that topology as
before, and otherwise MinBlockTime* and MaxTPS* add that many of the
synthetic organizations (0 to 30) to StableApproximateTier1CoreSets'
base 10. --tier-1-orgs-to-add 9 is the 19-organization topology that
--tier1-org-count 19 used to give.

The topology test now checks the added organizations directly: their
order and locations, that the base organizations are unchanged, and that
they reach named regions the base topology has no validator in, rather
than counting distinct locations.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
evaluateAt ran overlay-only (MIXED_PREGEN_*) and apply-mode candidates
down two branches that had drifted apart: only apply mode logged the
SLA after a loadgen failure, the metric snapshot was explained in one
branch, and a failed health check failed an overlay-only candidate but
aborted the whole mission in apply mode, although the doc says it
fails the candidate. Both now take the same steps: overlay-only mode
(if any), loadgen, one metric snapshot read before the health checks,
the inclusion cross-check (overlay-only only), the health checks, and
one verdict line. Every failure fails the candidate, as documented, and
the close-time SLA is always logged. The inclusion check returns its own
failure reason, so its message follows minInclusionFraction instead of
a hard-coded 95%.

Equal --min/--max-block-time-ms bounds now go through the same search
as a one-candidate list (closeTimeCandidates), still evaluated exactly
as given, instead of a separate branch.

MinBlockTimeTest now owns the every-validator decision: it is simply
MIXED_PREGEN_* load, so StellarKubeSpecs.LoadOnEveryValidator is gone,
RunMultiLoadgen and LoadGenPeers read the context's
pregenerateTxsPerValidator flag, and the account partitioning uses that
one condition and the active load-generating sets.

withOverlayOnlyMode moves to MissionTriggerTimerMixConsensus, its only
user. Also: return types on PartitionValidatorLoad, LoadgenPeerIndices
and PregenerationOptionsForPeer, no more "upstream" in comments, and a
note on why needsRecovery is a ref cell.

Tests: the partition tests derive their numbers (57 = 19 organizations
x 3 validators; 21,052 = 1,200,000 / 57, floored) instead of hard-coding
them, explain why a partial-second budget is rejected, drop a tally
that only re-checked the selection count, cover all three MIXED_PREGEN
modes, and add cases for which partition values are floored and which
are spread. The search test records evaluations in a ref cell instead
of a System.Collections.Generic.List.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…sured

Two changes to how a candidate is judged after its load.

Inclusion (overlay-only candidates). The cross-check passed when the
nodes' average count of transactions in ledgers reached 95% of the
offered load. But loadgen reports success only once every transaction
it submitted has been included in a closed ledger, and nodes that close
the same ledgers count the same transactions, so after a load the
network carried every node reports the same count, covering the offered
load. The 5% slack could only hide the discrepancy the check exists to
catch, and the average let one node's excess hide another's shortfall.

After a completed load every node must now report the same count,
covering the offered load; a node still closing the last loaded ledger
is re-read for up to two close times. Otherwise the candidate fails,
with each node's count logged: a node that fell further behind means
the network did not sustain the close time on every node, and a
shortfall on every node means the load was not carried. After a failed
load the candidate has already failed, so the counts are not read.

Unreadable metrics. The single evaluation path counted unreadable
metrics as a failed candidate. After a completed load that is a harness
or network problem, not a verdict: the candidate was never measured,
and failing it would let the search raise its lower bound on no
evidence and report a close time that is too high. As on main, the
mission aborts instead. After a failed load, usually a node dropping
out, which also leaves its metrics unreadable, the read failure is only
logged and the search continues.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 2, 2026 18:02
@SirTyson
SirTyson force-pushed the min-block-time-methodology branch from 992c2b9 to b3761af Compare October 2, 2026 18:02
@SirTyson

SirTyson commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for your feedback! I've addressed it in the following commits:

4db25ce Keep per-validator pregeneration failures fatal Fixed getInitCommands to only start core pods that actually make it through setup cleanly

ee4b9ba Use --tier-1-orgs-to-add for the synthetic tier 1 topology Without --pubnet-data it adds that many synthetic organizations (0 to 30) to the default 10

e99e328 Unify the evaluation paths for both overlay only and full apply load modes

b3761af Cleanup inclusion check

@SirTyson
SirTyson requested a review from drebelsky October 2, 2026 18:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

// covering the `offered` load. Exposed for unit tests.
let inclusionAgrees (offered: int) (included: ('node * float) list) : bool =
match included |> List.map snd |> List.distinct with
| [ txs ] -> txs >= float offered
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants