Skip to content

pack: only keep the 16 highest-tipping early bundles - #12589

Open
topointon-jump wants to merge 1 commit into
mainfrom
firedancer-stale-bundles
Open

topointon-jump wants to merge 1 commit into
mainfrom
firedancer-stale-bundles

Conversation

@topointon-jump

Copy link
Copy Markdown
Contributor

The vast majority of bundles that arrive before our leader rotation are spam. We don't want to waste the first slot trying to execute these. However, there are occasionally some high-tipping ones. Therefore, a simple heuristic is to keep only the 16 highest-tipping bundles that arrive before our leader rotation. 16 was chosen after looking at a lot of pack captures.

Copilot AI balanced review requested due to automatic review settings October 29, 2026 03:09
@topointon-jump
topointon-jump force-pushed the firedancer-stale-bundles branch from 91994cc to 32aa493 Compare October 29, 2026 03:14
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
 ┌─ ⚡ PERF · 5fc9f9c vs main@7695701 ─────────────────────────────────
 │ SUITE                               BASELINE          NEW         Δ
 │ replay tps, mainnet               13,689 tps   13,688 tps  ·  -0.01%
 │ bench tps, localnet              672,995 tps  668,375 tps  ·  -0.69%
 │ snapshot load, testnet                4.94 s       4.94 s  ·  +0.06%
 │ epoch boundary, mainnet             160.4 ms     162.3 ms  ·  +1.19%
!│ mem total, mainnet                147.34 GiB   147.34 GiB  ▲   0.00%
!│ mem total, testnet                 84.66 GiB    84.66 GiB  ▲   0.00%
 │ clean compile, firedancer             3.15 s       3.14 s  ·  -0.32%
 │ binary size, firedancer             57.98 MB     57.99 MB  ·  +0.02%
 ├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 2 WARNINGS · 0 IMPROVED · 6 NOISE @@
 └─────────────────────────────────────────────────────────────────────
history · 8 pushes
 ┌─ HISTORY · Δ vs main, per push, newest first ──────────────────────────────────
 │ HEAD         TPS    BENCH     SNAP    EPOCH    MEM·M    MEM·T  COMPILE   BINARY
!│ 5fc9f9c   -0.01%   -0.69%   +0.06%   +1.19%    0.00%    0.00%   -0.32%   +0.02%
 │ bd73e45        …        …        …        …        …        …        …        …
!│ 515e114   -0.30%   +0.13%   -5.36%   -1.89%    0.00%    0.00%   +0.85%   +0.02%
+│ 2494746   +0.06%   +0.20%   +0.88%   -1.93%    0.00%    0.00%   -0.56%   +0.02%
+│ f0b1380   +0.28%   +0.53%   -0.15%   +0.02%    0.00%    0.00%   -0.77%   +0.02%
!│ 1181942   -0.08%   -0.24%   +1.06%   -0.45%    0.00%    0.00%   -0.07%   +0.02%
+│ 930cb71   +0.13%   +0.46%   +0.13%   +2.67%    0.00%    0.00%   +0.54%   +0.02%
+│ 32aa493   +0.67%   +0.05%   -0.89%   -0.17%    0.00%    0.00%   -0.87%   +0.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.

🟡 Changes recommended

An oversized early bundle can trigger an infinite eviction loop, while deferred-bundle metadata lookup can configure the wrong builder or commission.

4 open issues
What changed in this PR

This PR limits pack’s pre-rotation backlog to the 16 highest declared-tip bundles, aiming to reduce spam execution at the start of a leader rotation.

Changes:

  • Adds tip-ranked storage, admission, eviction, and fallback scheduling for early bundles.
  • Passes the leader rotation’s end time from the pack tile to classify arriving bundles.
  • Adds five metrics covering early-bundle retention and scheduling, with generated schemas and documentation.
File Description
src/​disco/​pack/​fd_pack.h Declares the rotation-end timestamp API.
src/​disco/​pack/​fd_pack.c Implements early-bundle ranking, capacity limits, scheduling, and accounting.
src/​disco/​pack/​fd_pack_tile.c Supplies the calculated leader rotation end time.
src/​disco/​metrics/​metrics.xml Defines five early-bundle metrics.
src/​disco/​metrics/​generated/​fd_metrics_sample_gen.c Adds the metrics to sample serialization.
src/​disco/​metrics/​generated/​fd_metrics_pack.h Adds generated metric offsets and definitions.
src/​disco/​metrics/​generated/​fd_metrics_all.c Registers the new metric metadata.
src/​disco/​events/​schema/​metrics_pack.json Adds event-storage fields for the metrics.
src/​disco/​events/​schema/​events.proto Adds protobuf fields for the metrics.
book/​api/​metrics-generated.md Documents the new metrics.

Review effort: Balanced

Comment thread src/disco/pack/fd_pack.c Outdated
Comment thread src/disco/pack/fd_pack.c
Comment thread src/disco/pack/fd_pack.c
Comment thread src/disco/pack/fd_pack.c
Copilot AI balanced review requested due to automatic review settings October 29, 2026 03:16

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.

🟡 Changes recommended

Metadata lookup does not follow the scheduler’s skip and fallback rules, allowing a selected bundle to execute with another bundle’s commission and tip configuration.

5 open issues

Review effort: Balanced

Comment thread src/disco/pack/fd_pack.c Outdated
Copilot AI balanced review requested due to automatic review settings October 29, 2026 16:06

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.

🔵 Needs a closer look

Full-pack bundle replacement can unnecessarily evict unrelated transactions, and valid transfers with trailing instruction data are incorrectly counted as zero-tip.

2 open issues
3 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 29, 2026 16:18

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.

🔵 Needs a closer look

The new top-16 bundle policy lacks regression coverage for tip-based rejection, whole-bundle eviction, count maintenance, and rotation-boundary bypass and scheduling priority.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@topointon-jump
topointon-jump force-pushed the firedancer-stale-bundles branch from 1181942 to f0b1380 Compare October 29, 2026 16:52
Copilot AI balanced review requested due to automatic review settings October 29, 2026 16:52

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.

🔵 Needs a closer look

The new 16-bundle admission policy lacks targeted regression tests for tip-based rejection and replacement, whole-bundle eviction, and fresh-bundle precedence across the rotation boundary.

No open issues 🎉

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 29, 2026 21:00

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.

🟡 Changes recommended

A rejected higher-tipping replacement can delete a valid buffered bundle before duplicate-nonce validation completes, leaving neither bundle queued.

1 open issue

Review effort: Balanced

Comment thread src/disco/pack/fd_pack.c
break;
} else {
ulong _delete_cnt = delete_transaction( pack, same_nonce, 0, 0 );
ulong _delete_cnt = delete_transaction( pack, same_nonce, 1, 0 );
Copilot AI balanced review requested due to automatic review settings October 29, 2026 21:10

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.

🔵 Needs a closer look

Replacement deletions occur before validation completes, allowing rejected incoming bundles to permanently discard valid retained bundles, while the new selection policy lacks regression coverage.

1 open issue

Review effort: Balanced

@topointon-jump
topointon-jump force-pushed the firedancer-stale-bundles branch from 515e114 to bd73e45 Compare October 29, 2026 23:44
Copilot AI balanced review requested due to automatic review settings October 29, 2026 23:44
@topointon-jump
topointon-jump force-pushed the firedancer-stale-bundles branch from bd73e45 to 5fc9f9c Compare October 29, 2026 23: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.

🔵 Needs a closer look

Rejected incoming bundles can permanently delete valid retained bundles before validation completes, and the new retention policy lacks boundary, tip-ranking, and ordering tests.

1 open issue

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 29, 2026 23:48

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.

🔵 Needs a closer look

Nonce-conflict evictions occur before all incoming-bundle checks pass, allowing rejected bundles to permanently remove valid retained bundles.

1 open issue

Review effort: Balanced

@topointon-jump
topointon-jump removed this pull request from stack #12620 October 30, 2026 02:52

This branch has not been deployed

No deployments
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.

2 participants