feat: add MEV earned per epoch in the ValidatorHistoryAccount #13

0xprames · 2024-01-08T02:35:59Z

I took a stab at #6 , saw it's been open for a few weeks

mainly out of wanting to learn, this is my first experience w/ Anchor, solana programs, instructions etc etc

I'd love a review to see if I thought about the issue correctly, but i'm happy to close it out or collaborate w/ the assignee of #6 to get this in

buffalu · 2024-01-08T02:41:45Z

programs/validator-history/src/state.rs

+        epoch: u16,
+        commission: u16,
+        mev_earned: u64,
+    ) -> Result<()> {
        // check if entry exists for the epoch


this logic is wrong. mentioned to @ebatsell in diff PR, but if you update epoch 500, then 501, then 500 you'll end up with two epoch 500 entries. the second 500 update should go back and find the first 500 entry

i deleted my prev comment on this thread - since it was incorrect

you're right, with the existing logic, in the case where we try and edit an epoch seen prev, it will create another entry for it.

i went ahead and pushed up a fix for that

high level:

the CircBuf struct should have at most one entry per epoch, and they should be laid out in ascending order

if for some reason an entry is missed, it's pretty messy to try to insert after the fact due to the need to shift every following epoch down by one.

a new entry should be appended if an entry doesn't exist for that epoch and it's greater than the most recent epoch (.last())

therefore the update pseudocode for all of these set_s should be something like:

if epoch > circ_buf.last().epoch { insert new entry } else { if let Some(entry) = circ_buf.find(epoch) { modify entry } }

yeah makes sense, should be taken care of for all the set_s now

programs/validator-history/src/state.rs

ebatsell · 2024-01-09T22:26:09Z

programs/validator-history/src/state.rs

+        epoch: u16,
+        commission: u16,
+        mev_earned: u64,
+    ) -> Result<()> {
        // check if entry exists for the epoch


high level:

the CircBuf struct should have at most one entry per epoch, and they should be laid out in ascending order

if for some reason an entry is missed, it's pretty messy to try to insert after the fact due to the need to shift every following epoch down by one.

a new entry should be appended if an entry doesn't exist for that epoch and it's greater than the most recent epoch (.last())

therefore the update pseudocode for all of these set_s should be something like:

if epoch > circ_buf.last().epoch { insert new entry } else { if let Some(entry) = circ_buf.find(epoch) { modify entry } }

ebatsell · 2024-01-09T22:27:58Z

keepers/validator-keeper/src/mev_commission.rs

@@ -122,7 +124,7 @@ impl UpdateInstruction for ValidatorMevCommissionEntry {
                signer: self.signer,
            }
            .to_account_metas(None),
-            data: validator_history::instruction::UpdateMevCommission {}.data(),
+            data: validator_history::instruction::UpdateMevCommission { epoch: self.epoch }.data(),


Merkle roots for a given epoch are uploaded typically within a few hours after the epoch ends, so to account for this, the keeper will need to submit UpdateMevCommission instructions for the previous epoch after its upload has completed. could achieve this by adding a separate loop in the main that submits these at say, the halfway point of the next epoch. Or better yet, continually checks the tip distribution accounts to see which merkle_roots are not None

for sure, lmk if what i have works

i added a update_mev_earned loop similar to the existing update_mev_comission with the main difference that instead of just looking for existing TDAs, it attempts to TipDistributionAccount::try_deserialize on the account.data.data received from the gMA call and and check for Some(merkle_root) = tda.merkle_root

ebatsell · 2024-01-09T23:37:24Z

programs/validator-history/src/state.rs

    pub padding0: u8,
    // 0 if not a superminority validator, 1 if superminority validator
    pub is_superminority: u8,
    // rank of validator by stake amount
    pub rank: u32,
    // Most recent updated slot for epoch credits and commission
    pub vote_account_last_update_slot: u64,
-    pub padding1: [u8; 88],
+    // MEV earned
+    pub mev_earned: u64,


in the interest of using the remaining space here wisely, I think it'd be best to represent this as a fixed point decimal. if this is 4 bytes with 2 decimals of precision that gives us a max MEV earned of 42949672.95 SOL for 1 validator for 1 epoch which won't happen for.. a long time 😄

for sure, lmk if what i pushed up is correct

the internal u32 representation / 100 should give the ~~max~~ mev earned in sol with 2 dec of precision

ebatsell

Since we're doing more than just setting mev_commission in this instruction, I think we should also rename the instruction to copy_tip_distribution_account

ebatsell · 2024-01-11T22:39:41Z

programs/validator-history/src/state.rs

    pub padding0: u8,
    // 0 if not a superminority validator, 1 if superminority validator
    pub is_superminority: u8,
    // rank of validator by stake amount
    pub rank: u32,
    // Most recent updated slot for epoch credits and commission
    pub vote_account_last_update_slot: u64,
-    pub padding1: [u8; 88],
+    // MEV earned


Suggested change

// MEV earned

// MEV earned, stored as 1/100th SOL. mev_earned = 100 means 1 SOL earned

ebatsell · 2024-01-11T22:42:27Z

programs/validator-history/src/utils.rs

@@ -4,6 +4,16 @@ pub fn cast_epoch(epoch: u64) -> u16 {
    (epoch % u16::MAX as u64).try_into().unwrap()
 }

+pub fn fixed_point_sol(lamports: u64) -> u32 {
+    // convert to sol
+    let mut sol = lamports as f64 / 1000000000.00;


can use anchor_lang::solana_program::native_token::lamports_to_sol

programs/validator-history/src/utils.rs

keepers/validator-keeper/src/mev_commission.rs

ebatsell · 2024-01-11T23:38:33Z

programs/validator-history/src/instructions/update_mev_commission.rs

    let mut validator_history_account = ctx.accounts.validator_history_account.load_mut()?;

    let mut tda_data: &[u8] = &ctx.accounts.tip_distribution_account.try_borrow_data()?;

    let tip_distribution_account = TipDistributionAccount::try_deserialize(&mut tda_data)?;
    let mev_commission_bps = tip_distribution_account.validator_commission_bps;
-    let epoch = cast_epoch(Clock::get()?.epoch);


We need to do a check here and anywhere else epoch is passed in that epoch is not greater than Clock::get().epoch

done, the other 2 instructions (update_stake_history and backfill_total_blocks) seem to have the check

ebatsell · 2024-01-12T00:02:21Z

programs/validator-history/src/state.rs

+                        .filter(|entry| entry.epoch == epoch)
+                        .for_each(|entry| {


this implies there may be multiple entries with the same epoch in the CircBuf, which shouldn't be possible given how we're inserting new entries - would prefer to use .find(|entry| entry.epoch == epoch) here instead of .filter().for_each(..)

makes sense

0xprames · 2024-01-19T04:50:55Z

Since we're doing more than just setting mev_commission in this instruction, I think we should also rename the instruction to copy_tip_distribution_account

done - UpdateMevComission -> CopyTipDistributionAccount

ebatsell · 2024-01-20T00:28:12Z

@0xprames looks great, once all tests and checks are passing I'll approve

buffalu

mostly stylistic comments!

buffalu · 2024-01-11T15:43:59Z

keepers/validator-keeper/src/main.rs

+        // Continuously runs throughout an epoch, polling for tip distribution accounts from the prev epoch with uploaded merkle roots
+        // and submitting update_mev_earned (technically update_mev_comission) txs when the uploaded merkle roots are detected
+        match update_mev_earned(
+            client.clone(),


use &client and &keypair here to avoid clones

buffalu · 2024-01-11T15:46:37Z

keepers/validator-keeper/src/mev_commission.rs

@@ -190,6 +193,73 @@ pub async fn update_mev_commission(
    submit_result.map_err(|(e, stats)| (e.into(), stats))
 }

+pub async fn update_mev_earned(
+    client: Arc<RpcClient>,


can use & here and on keypair

makes sense, i def should borrow more than move into when i can

buffalu · 2024-01-20T02:10:04Z

keepers/validator-keeper/src/main.rs

@@ -369,5 +406,12 @@ async fn main() {
        args.interval,
    ));

+    tokio::spawn(mev_earned_loop(
+        Arc::clone(&client),


this can be client.clone()

done, modified in the existing code as well

buffalu · 2024-01-20T02:10:47Z

keepers/validator-keeper/src/mev_commission.rs

+        .map(|vote_account| {
+            ValidatorMevCommissionEntry::new(
+                vote_account,
+                epoch - 1, // TDA derived from the prev epoch since the merkle roots are uploaded shortly after rollover


ultra nit: saturating_sub for epoch 0

makes sense

buffalu · 2024-01-20T02:11:05Z

keepers/validator-keeper/src/mev_commission.rs

+        .collect::<Vec<ValidatorMevCommissionEntry>>();
+
+    let uploaded_merkleroot_entries =
+        get_entries_with_uploaded_merkleroot(client.clone(), &entries)


use &client

buffalu · 2024-01-20T02:11:22Z

keepers/validator-keeper/src/mev_commission.rs

@@ -215,3 +286,33 @@ async fn get_existing_entries(
    // Fetch existing tip distribution accounts for this epoch
    Ok(result)
 }
+
+async fn get_entries_with_uploaded_merkleroot(
+    client: Arc<RpcClient>,


keepers/validator-keeper/src/mev_commission.rs

buffalu · 2024-01-20T02:14:46Z

programs/validator-history/src/instructions/copy_tip_distribution_account.rs

@@ -48,16 +49,25 @@ pub struct UpdateMevCommission<'info> {
    pub signer: Signer<'info>,
 }

-pub fn handler(ctx: Context<UpdateMevCommission>) -> Result<()> {
+pub fn handler(ctx: Context<CopyTipDistributionAccount>, epoch: u64) -> Result<()> {
+    // Cannot set stake for future epochs


buffalu requested a review from ebatsell January 8, 2024 02:38

buffalu reviewed Jan 8, 2024

View reviewed changes

0xprames force-pushed the master branch from e0776a0 to c152cbf Compare January 8, 2024 04:29

ebatsell reviewed Jan 9, 2024

View reviewed changes

0xprames force-pushed the master branch from c152cbf to b36d1bc Compare January 11, 2024 06:34

ebatsell reviewed Jan 11, 2024

View reviewed changes

ebatsell reviewed Jan 12, 2024

View reviewed changes

0xprames added 5 commits January 18, 2024 23:48

feat: add MEV earned per epoch in the ValidatorHistoryAccount

3ec2430

fix find entry by epoch logic in set_mev_commission

d5557cc

review comments

14ed111

review comments

3566e93

review comments + clippy

e3306cf

0xprames force-pushed the master branch from b36d1bc to e3306cf Compare January 19, 2024 04:48

cargo fmt

4a7e60a

buffalu reviewed Jan 20, 2024

View reviewed changes

0xprames added 4 commits January 20, 2024 11:45

review comments

63faa09

clippy

4bf26fa

add idl

5bb4c90

idl with anchor version 0.28

1991333

ebatsell approved these changes Jan 26, 2024

View reviewed changes

ebatsell merged commit 6236581 into jito-foundation:master Jan 26, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add MEV earned per epoch in the ValidatorHistoryAccount #13

feat: add MEV earned per epoch in the ValidatorHistoryAccount #13

0xprames commented Jan 8, 2024

buffalu Jan 8, 2024

0xprames Jan 9, 2024

ebatsell Jan 9, 2024

0xprames Jan 11, 2024

ebatsell Jan 9, 2024

ebatsell Jan 9, 2024

0xprames Jan 11, 2024

ebatsell Jan 11, 2024

ebatsell Jan 9, 2024

0xprames Jan 11, 2024 •

edited

Loading

ebatsell left a comment

ebatsell Jan 11, 2024

0xprames Jan 19, 2024

ebatsell Jan 11, 2024

0xprames Jan 19, 2024

ebatsell Jan 11, 2024

0xprames Jan 19, 2024

ebatsell Jan 12, 2024

0xprames Jan 19, 2024

0xprames commented Jan 19, 2024

ebatsell commented Jan 20, 2024

buffalu left a comment

buffalu Jan 11, 2024

buffalu Jan 11, 2024

0xprames Jan 20, 2024

buffalu Jan 20, 2024

0xprames Jan 20, 2024

buffalu Jan 20, 2024

0xprames Jan 20, 2024

buffalu Jan 20, 2024

buffalu Jan 20, 2024

buffalu Jan 20, 2024

0xprames Jan 20, 2024

	// MEV earned
	// MEV earned, stored as 1/100th SOL. mev_earned = 100 means 1 SOL earned

feat: add MEV earned per epoch in the ValidatorHistoryAccount #13

feat: add MEV earned per epoch in the ValidatorHistoryAccount #13

Conversation

0xprames commented Jan 8, 2024

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

0xprames Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

ebatsell left a 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

Choose a reason for hiding this comment

0xprames commented Jan 19, 2024

ebatsell commented Jan 20, 2024

buffalu left a 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

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

0xprames Jan 11, 2024 •

edited

Loading