WIP: generating rewards certs and adding them to the block footer #582

akhi3030 · 2025-11-04T14:20:23Z

See https://docs.google.com/document/d/1jyycl6EwP8eVqdrYj3tmFHfG5U86SPr91WXG3DvNyWQ/edit?tab=t.0 for a design doc

This is a WIP of how we can generate and add rewards certificates in the block footer.

The approach taken here completely bypasses the consensus pool which means that for the votes relevant for rewards, we are copying them an additional time and aggregating them into certificates an additional time.

akhi3030 · 2025-11-12T14:12:57Z

core/src/bls_sigverify/bls_sigverifier.rs

+
+                    // consensus pool does not need votes for slots other than root slot however the rewards container may still need them.
                    if vote_message.vote.slot() <= root_bank.slot() {
+                        // the only module that takes a write lock on consensus_rewards is this one and it does not take the write lock while it is verifying votes so this should never block.


feedback from Brennan: maybe we can do this inline if we can demonstrate that we have enough compute resources to handle the additional verification. This will also require ensuring that the dedup can handle the DDoS vectors we are worried about.

akhi3030 · 2025-11-12T14:15:00Z

votor/src/consensus_rewards.rs

+    }
+
+    /// Adds received [`VoteMessage`]s from other nodes.
+    pub fn add_vote_message(&mut self, root_slot: Slot, vote: VoteMessage) {


Potential optimisation: if we look up how many validators we have for this epoch, then we can detect here if we have received votes from all the validators and pre-generate the certificates in this function instead of build_rewards_certs. build_rewards_certs is more in the critical path than this function is. Not sure how often we will actually hit this case though.

steviez

Briefly glanced. I still need to work through bls sigverifier to be able to give more in depth reviews on approach. Pretty strapped for time at the moment tho

steviez · 2025-11-18T04:22:47Z

votor/src/consensus_rewards/entry.rs

+    /// map from validator rank to the skip vote.
+    skip: BTreeMap<u16, VoteMessage>,
+    /// notar votes are indexed by block id as different validators may vote for different blocks.
+    notar: BTreeMap<Hash, BTreeMap<u16, VoteMessage>>,


pub struct ConsensusRewards { /// Per [`Slot`], stores skip and notar votes. votes: BTreeMap<Slot, Entry>, /// Stores the latest pubkey for the current node. cluster_info: Arc<ClusterInfo>, /// Stores the leader schedules. leader_schedule_cache: Arc<LeaderScheduleCache>, }

I have some concerns about allocations, this is several layers deep of BTreeMap nesting. It looks like we'd also be doing this with a lock held

Agreed. What if I switch to HashMap and use with_capacity()? In almost all of the instances, we have a good idea of what the expected size of the containers will be.

replaced most of the btreemaps with hashmaps now. One remains where we are actually relying on the ordering or keys so not so easy to remove.

steviez · 2025-11-18T04:27:26Z

core/src/block_creation_loop.rs

+    let handle = std::thread::spawn(move || {
+        // XXX: how to look up the slot.
+        let slot = u64::MAX;
+        consensus_rewards.read().build_rewards_certs(slot)
+    });


I know this isn't final, but spawning unnamed threads like this is not how we typically like to do stuff in parallel

absolutely, this was just to show what I propose to do. Is there an example that I can follow on how I could do something like this?

steviez · 2025-11-18T04:40:59Z

core/src/block_creation_loop.rs

        replay_stage::{Finalizer, ReplayStage},
    },
    crossbeam_channel::Receiver,
+    parking_lot::RwLock as PLRwLock,


Why this over std::sync::RwLock ?

A couple of reasons:

My understanding is that parking lot is fairer to writers and ensures that writers do not starve. In particular, if there is a writer waiting, then additional readers also have to wait

for small critical sections, benchmarks seems to suggest that parking lot does better.

akhi3030

thanks for the feedback Steve. I'll push a change to use HashMap and with_capacity to minimise allocations.

akhi3030 · 2025-11-18T10:28:33Z

core/src/block_creation_loop.rs

        replay_stage::{Finalizer, ReplayStage},
    },
    crossbeam_channel::Receiver,
+    parking_lot::RwLock as PLRwLock,


A couple of reasons:

My understanding is that parking lot is fairer to writers and ensures that writers do not starve. In particular, if there is a writer waiting, then additional readers also have to wait

for small critical sections, benchmarks seems to suggest that parking lot does better.

akhi3030 · 2025-11-18T10:29:24Z

core/src/block_creation_loop.rs

+    let handle = std::thread::spawn(move || {
+        // XXX: how to look up the slot.
+        let slot = u64::MAX;
+        consensus_rewards.read().build_rewards_certs(slot)
+    });


absolutely, this was just to show what I propose to do. Is there an example that I can follow on how I could do something like this?

akhi3030 · 2025-11-18T10:41:08Z

votor/src/consensus_rewards/entry.rs

+    /// map from validator rank to the skip vote.
+    skip: BTreeMap<u16, VoteMessage>,
+    /// notar votes are indexed by block id as different validators may vote for different blocks.
+    notar: BTreeMap<Hash, BTreeMap<u16, VoteMessage>>,


Agreed. What if I switch to HashMap and use with_capacity()? In almost all of the instances, we have a good idea of what the expected size of the containers will be.

This allows us to use `with_capacity` which hopefully prevents allocations

akhi3030 · 2025-11-24T07:56:48Z

core/src/block_creation_loop.rs

    block_timer: Instant,
    block_timeout: Duration,
 ) -> Result<(), PohRecorderError> {
+    // Taking a read lock on consensus_rewards can contend with the write lock in bls_sigverifier.


Feedback from @AshwinSekar: #607 (comment)

@AshwinSekar : I've reworked the PR to use channels and a dedicated thread now. There is also a "stateless" version of wants_vote that we could call from the BLS sigverifier to do some filtering and avoid cloning all votes. I'll add support for that as a separate commit.

akhi3030 force-pushed the akhi3030/rewards-in-block-footer branch from 76d6e7d to cfe8f88 Compare November 4, 2025 14:26

akhi3030 changed the title ~~wip~~ POC: generating rewards certs and adding them to the block footer Nov 4, 2025

akhi3030 force-pushed the akhi3030/rewards-in-block-footer branch 23 times, most recently from c9cb68d to 3170fc4 Compare November 10, 2025 15:05

wip

f10419e

akhi3030 force-pushed the akhi3030/rewards-in-block-footer branch from 3170fc4 to f10419e Compare November 10, 2025 15:20

akhi3030 changed the title ~~POC: generating rewards certs and adding them to the block footer~~ WIP: generating rewards certs and adding them to the block footer Nov 10, 2025

akhi3030 requested a review from steviez November 12, 2025 13:41

akhi3030 commented Nov 12, 2025

View reviewed changes

akhi3030 added 2 commits November 14, 2025 07:46

uncomment some code to prepare for merging master

d6ae39e

Merge branch 'master' into akhi3030/rewards-in-block-footer

0e05f62

steviez reviewed Nov 18, 2025

View reviewed changes

akhi3030 commented Nov 18, 2025

View reviewed changes

akhi3030 added 2 commits November 18, 2025 13:09

clippy fixes

bbe27dd

replace some btreemaps with hashmaps

ddec9ae

This allows us to use `with_capacity` which hopefully prevents allocations

akhi3030 requested a review from steviez November 18, 2025 13:58

akhi3030 mentioned this pull request Nov 24, 2025

wip: pay rewards #607

Draft

akhi3030 commented Nov 24, 2025

View reviewed changes

use channels and threads instead of locks

1785420

akhi3030 force-pushed the akhi3030/rewards-in-block-footer branch from 15dc61a to 1785420 Compare November 25, 2025 06:41

WIP: generating rewards certs and adding them to the block footer #582

Are you sure you want to change the base?

WIP: generating rewards certs and adding them to the block footer #582

Uh oh!

Conversation

akhi3030 commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steviez left a comment

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!

akhi3030 left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

akhi3030 commented Nov 4, 2025 •

edited

Loading