genesis: create EpochRewards at epoch 0 #6125

2501babe · 2025-05-06T13:44:26Z

Problem

the bpf stake program depends on the existence of EpochRewards to function, but this account does not exist until epoch 1. this means:

all stake operations fail on solana-test-validator in the first epoch
all stake operations would fail on a brand new cluster in the first epoch

Summary of Changes

create a blank SysvarEpochRewards1111111111111111111111111 account at genesis

closes #4928

codecov-commenter · 2025-05-06T14:40:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.8%. Comparing base (636f6c1) to head (de43942).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #6125     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         848      849      +1     
  Lines      378808   378817      +9     
=========================================
- Hits       313796   313781     -15     
- Misses      65012    65036     +24

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mergify · 2025-05-23T07:29:33Z

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

2501babe · 2025-05-23T07:37:05Z

runtime/src/genesis_utils.rs

+    let epoch_rewards_account = AccountSharedData::create(
+        1,
+        vec![0; EpochRewards::size_of()],
+        sysvar::id(),
+        false,
+        u64::MAX,
+    );
+    initial_accounts.push((epoch_rewards::id(), epoch_rewards_account));


based on watching the account on solana-test-validator there is no reason to get clever with the lamports here because it gets reset to the rent-exempt minimum at the first epoch boundary anyway. we could hardcode the value 1454640 but i avoided getting it via rent.minimum_balance() because lamports_per_signature is 0 in some tests, so im wary using Rent during genesis is brittle

We're using rent during genesis when creating the stake config:

agave/programs/stake/src/config.rs

Lines 46 to 50 in b88655c

pub fn add_genesis_account(genesis_config: &mut GenesisConfig) -> u64 {

let mut account = create_config_account(vec![], &Config::default(), 0);

let lamports = genesis_config.rent.minimum_balance(account.data().len());

account.set_lamports(lamports.max(1));

So let's do that here as well.

Also, is there a reason to add the new account here as opposed to here?

agave/programs/stake/src/lib.rs

Lines 16 to 18 in b88655c

pub fn add_genesis_accounts(genesis_config: &mut GenesisConfig) -> u64 {

config::add_genesis_account(genesis_config)

}

That way, totally new Solana networks using solana-genesis will also get the account:

agave/genesis/src/main.rs

Line 750 in b88655c

solana_stake_program::add_genesis_accounts(&mut genesis_config);

i moved it to the stake program function, i didnt know there was a separate genesis cli that didnt use genesis utils

2501babe · 2025-05-23T07:41:44Z

i tagged this to backport because it has no effect on runtime behavior for any cluster (so it is safe), the only impact is to make the 2.2 version of solana-test-validator behave properly when running the bpf stake program, which is likely to activate while mainnet is on 2.2

joncinque

Just a comment about where to put the function, the logic looks good!

joncinque · 2025-05-29T13:16:24Z

runtime/src/genesis_utils.rs

+    let epoch_rewards_account = AccountSharedData::create(
+        1,
+        vec![0; EpochRewards::size_of()],
+        sysvar::id(),
+        false,
+        u64::MAX,
+    );
+    initial_accounts.push((epoch_rewards::id(), epoch_rewards_account));


We're using rent during genesis when creating the stake config:

agave/programs/stake/src/config.rs

Lines 46 to 50 in b88655c

pub fn add_genesis_account(genesis_config: &mut GenesisConfig) -> u64 {

let mut account = create_config_account(vec![], &Config::default(), 0);

let lamports = genesis_config.rent.minimum_balance(account.data().len());

account.set_lamports(lamports.max(1));

So let's do that here as well.

Also, is there a reason to add the new account here as opposed to here?

agave/programs/stake/src/lib.rs

Lines 16 to 18 in b88655c

pub fn add_genesis_accounts(genesis_config: &mut GenesisConfig) -> u64 {

config::add_genesis_account(genesis_config)

}

That way, totally new Solana networks using solana-genesis will also get the account:

agave/genesis/src/main.rs

Line 750 in b88655c

solana_stake_program::add_genesis_accounts(&mut genesis_config);

joncinque · 2025-05-29T13:17:12Z

runtime/src/bank/partitioned_epoch_rewards/mod.rs

-                if slot == SLOTS_PER_EPOCH {
-                    // cap should increase because of new epoch rewards
-                    assert!(post_cap > pre_cap);
-                } else {
-                    assert_eq!(post_cap, pre_cap);
-                }


Very nice! It's great to get rid of these hacks

mergify · 2025-06-02T08:10:16Z

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

2501babe · 2025-06-02T08:45:58Z

runtime/src/bank/partitioned_epoch_rewards/mod.rs

+        genesis_config
+            .accounts
+            .get_mut(&epoch_rewards::id())
+            .unwrap()
+            .set_lamports(1);


this seems fine to me to prevent EpochRewards from being immediately deallocated instead of adding back the slot == SLOTS_PER_EPOCH hack. these tests use Rent::free() because there are rent-paying accounts so using Rent::default() would mess up the capitalization() checks

That makes sense to me. Can you just add a comment to explain the rationale since we'll probably forget?

alright i understand everything now

test-validator uses create_genesis_config_with_leader_ex_no_features() directly with configured Rent (and its "no fees" variant is 1 lamport per byte-year rather than 0) so it creates a rent-exempt EpochRewards. the failing runtime test uses a create_genesis_config_with_leader_ex_no_features() wrapper with Rent::free() which sets 0 lamports per byte-year just like the partitioned epoch rewards tests, so EpochRewards is created with 0 lamports. then that test compares the total bytes allocated to the total bytes of all loadable accounts and gets a mismatch because EpochRewards still exists and is not garbage-collected (because there is no rent) but does not "exist" from the point of view of get_account() (because it has zero lamports)

instead of test hacks i realized i could just create EpochRewards with the max of rent exemption and 1, made this change, then rechecked StakeConfig because it should trigger this runtime test too... and i had failed to notice account.set_lamports(lamports.max(1)); which is subtly wrong because it returns the wrong value, but nothing checks it

tldr it is just test setup but there is an easy fix with prior art that requires no hacks

joncinque

Looks good to me, just needs a little comment on the test change

joncinque · 2025-06-02T09:48:23Z

runtime/src/bank/partitioned_epoch_rewards/mod.rs

+        genesis_config
+            .accounts
+            .get_mut(&epoch_rewards::id())
+            .unwrap()
+            .set_lamports(1);


That makes sense to me. Can you just add a comment to explain the rationale since we'll probably forget?

joncinque · 2025-06-02T09:50:17Z

Also, test failure looks legit

joncinque

Ah right, I missed that too 😞 but this looks good to me!

(cherry picked from commit 8aaa0c7) # Conflicts: # programs/stake/src/lib.rs # runtime/src/bank/partitioned_epoch_rewards/mod.rs

(cherry picked from commit 8aaa0c7)

) genesis: create EpochRewards at epoch 0 (#6125) (cherry picked from commit 8aaa0c7) Co-authored-by: hana <81144685+2501babe@users.noreply.github.com>

2501babe force-pushed the 20250506_tesval_rewards branch from 74024a6 to 66624cc Compare May 6, 2025 13:45

2501babe self-assigned this May 6, 2025

2501babe force-pushed the 20250506_tesval_rewards branch 2 times, most recently from 59dd499 to 7cadcb0 Compare May 22, 2025 01:12

2501babe marked this pull request as ready for review May 23, 2025 07:29

2501babe added the v2.2 label May 23, 2025

2501babe requested a review from joncinque May 23, 2025 07:29

2501babe commented May 23, 2025

View reviewed changes

joncinque reviewed May 29, 2025

View reviewed changes

2501babe force-pushed the 20250506_tesval_rewards branch 2 times, most recently from 1711da1 to c37b620 Compare June 2, 2025 08:34

test-validator: create EpochRewards at epoch 0

05a6cc4

2501babe force-pushed the 20250506_tesval_rewards branch from c37b620 to 05a6cc4 Compare June 2, 2025 08:43

2501babe commented Jun 2, 2025

View reviewed changes

joncinque reviewed Jun 2, 2025

View reviewed changes

2501babe marked this pull request as draft June 2, 2025 09:54

fix

de43942

2501babe marked this pull request as ready for review June 2, 2025 10:28

joncinque approved these changes Jun 2, 2025

View reviewed changes

2501babe changed the title ~~test-validator: create EpochRewards at epoch 0~~ genesis: create EpochRewards at epoch 0 Jun 2, 2025

2501babe merged commit 8aaa0c7 into anza-xyz:master Jun 2, 2025
47 checks passed

2501babe deleted the 20250506_tesval_rewards branch June 2, 2025 11:41

mergify bot pushed a commit that referenced this pull request Jun 2, 2025

genesis: create EpochRewards at epoch 0 (#6125)

a2acc4f

(cherry picked from commit 8aaa0c7) # Conflicts: # programs/stake/src/lib.rs # runtime/src/bank/partitioned_epoch_rewards/mod.rs

mergify bot mentioned this pull request Jun 2, 2025

v2.2: genesis: create EpochRewards at epoch 0 (backport of #6125) #6374

Merged

2501babe added a commit that referenced this pull request Jun 2, 2025

genesis: create EpochRewards at epoch 0 (#6125)

cb948c9

(cherry picked from commit 8aaa0c7)

2501babe added a commit that referenced this pull request Jun 3, 2025

genesis: create EpochRewards at epoch 0 (#6125)

a8f65d6

(cherry picked from commit 8aaa0c7)

mircea-c pushed a commit to mircea-c/agave that referenced this pull request Jun 12, 2025

genesis: create EpochRewards at epoch 0 (anza-xyz#6125)

b9d9da2

mircea-c added a commit to mircea-c/agave that referenced this pull request Jun 12, 2025

genesis: create EpochRewards at epoch 0 (anza-xyz#6125)

cf64be8

	pub fn add_genesis_account(genesis_config: &mut GenesisConfig) -> u64 {
	let mut account = create_config_account(vec![], &Config::default(), 0);
	let lamports = genesis_config.rent.minimum_balance(account.data().len());

	account.set_lamports(lamports.max(1));

	pub fn add_genesis_accounts(genesis_config: &mut GenesisConfig) -> u64 {
	config::add_genesis_account(genesis_config)
	}

genesis: create EpochRewards at epoch 0 #6125

genesis: create EpochRewards at epoch 0 #6125

Uh oh!

Conversation

2501babe commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

codecov-commenter commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mergify bot commented May 23, 2025

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!

2501babe commented May 23, 2025

Uh oh!

joncinque 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!

mergify bot commented Jun 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2501babe Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joncinque commented Jun 2, 2025

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

2501babe commented May 6, 2025 •

edited

Loading

codecov-commenter commented May 6, 2025 •

edited

Loading

2501babe Jun 2, 2025 •

edited

Loading