From ce0a1397c1122055d65f59f8e845422d7cf83af6 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 12:16:39 -0500 Subject: [PATCH 01/10] Miscellaneous fixes --- keepers/validator-keeper/src/gossip.rs | 3 +- programs/validator-history/src/errors.rs | 2 + .../src/instructions/backfill_total_blocks.rs | 2 +- .../src/instructions/copy_cluster_info.rs | 3 +- .../instructions/copy_gossip_contact_info.rs | 47 ++++++++++++++++--- .../copy_tip_distribution_account.rs | 2 +- .../src/instructions/copy_vote_account.rs | 2 +- .../src/instructions/update_stake_history.rs | 7 ++- programs/validator-history/src/state.rs | 18 ++++--- programs/validator-history/src/utils.rs | 16 +++++-- tests/tests/test_cluster_history.rs | 6 +-- tests/tests/test_gossip.rs | 14 ++++-- utils/validator-history-cli/Cargo.toml | 2 +- 13 files changed, 91 insertions(+), 33 deletions(-) diff --git a/keepers/validator-keeper/src/gossip.rs b/keepers/validator-keeper/src/gossip.rs index 2756fc31..c4270704 100644 --- a/keepers/validator-keeper/src/gossip.rs +++ b/keepers/validator-keeper/src/gossip.rs @@ -128,7 +128,8 @@ impl GossipEntry { validator_history_account: self.validator_history_account, vote_account: self.vote_account, instructions: solana_program::sysvar::instructions::id(), - signer: self.signer, + config: self.config, + oracle_authority: self.signer, } .to_account_metas(None), data: validator_history::instruction::CopyGossipContactInfo {}.data(), diff --git a/programs/validator-history/src/errors.rs b/programs/validator-history/src/errors.rs index 4d0c4d52..b2679c31 100644 --- a/programs/validator-history/src/errors.rs +++ b/programs/validator-history/src/errors.rs @@ -26,4 +26,6 @@ pub enum ValidatorHistoryError { ArithmeticError, #[msg("Slot history sysvar is not containing expected slots")] SlotHistoryOutOfDate, + #[msg("Epoch larger than 65535, cannot be stored")] + EpochTooLarge, } diff --git a/programs/validator-history/src/instructions/backfill_total_blocks.rs b/programs/validator-history/src/instructions/backfill_total_blocks.rs index 3fb1a1e4..becea1f2 100644 --- a/programs/validator-history/src/instructions/backfill_total_blocks.rs +++ b/programs/validator-history/src/instructions/backfill_total_blocks.rs @@ -22,7 +22,7 @@ pub fn handle_backfill_total_blocks( ) -> Result<()> { let mut cluster_history_account = ctx.accounts.cluster_history_account.load_mut()?; - let epoch = cast_epoch(epoch); + let epoch = cast_epoch(epoch)?; // Need to backfill in ascending order when initially filling in the account otherwise entries will be out of order if !cluster_history_account.history.is_empty() diff --git a/programs/validator-history/src/instructions/copy_cluster_info.rs b/programs/validator-history/src/instructions/copy_cluster_info.rs index 3389412f..32c9dd28 100644 --- a/programs/validator-history/src/instructions/copy_cluster_info.rs +++ b/programs/validator-history/src/instructions/copy_cluster_info.rs @@ -32,9 +32,10 @@ pub fn handle_copy_cluster_info(ctx: Context) -> Result<()> { let clock = Clock::get()?; - let epoch = cast_epoch(clock.epoch); + let epoch = cast_epoch(clock.epoch)?; let epoch_start_timestamp = cast_epoch_start_timestamp(clock.epoch_start_timestamp); + // Sets the slot history for the previous epoch, since the current epoch is not yet complete. if epoch > 0 { cluster_history_account diff --git a/programs/validator-history/src/instructions/copy_gossip_contact_info.rs b/programs/validator-history/src/instructions/copy_gossip_contact_info.rs index d1215c15..2ea906ea 100644 --- a/programs/validator-history/src/instructions/copy_gossip_contact_info.rs +++ b/programs/validator-history/src/instructions/copy_gossip_contact_info.rs @@ -2,12 +2,34 @@ use anchor_lang::{ prelude::*, solana_program::{self, clock::Clock, pubkey::Pubkey, sysvar}, }; +use bytemuck::{from_bytes, Pod, Zeroable}; use crate::{ - crds_value::CrdsData, errors::ValidatorHistoryError, state::ValidatorHistory, utils::cast_epoch, + crds_value::CrdsData, errors::ValidatorHistoryError, state::ValidatorHistory, + utils::cast_epoch, Config, }; use validator_history_vote_state::VoteStateVersions; +// Structs and constants copied from solana_sdk::ed25519_instruction. Copied in order to make fields public. Compilation issues hit when importing solana_sdk +#[derive(Default, Debug, Copy, Clone, Zeroable, Pod, Eq, PartialEq)] +#[repr(C)] +pub struct Ed25519SignatureOffsets { + signature_offset: u16, // offset to ed25519 signature of 64 bytes + signature_instruction_index: u16, // instruction index to find signature + pub public_key_offset: u16, // offset to public key of 32 bytes + public_key_instruction_index: u16, // instruction index to find public key + pub message_data_offset: u16, // offset to start of message data + message_data_size: u16, // size of message data + message_instruction_index: u16, // index of instruction data to get message data +} + +pub const PUBKEY_SERIALIZED_SIZE: usize = 32; +pub const SIGNATURE_SERIALIZED_SIZE: usize = 64; +pub const SIGNATURE_OFFSETS_SERIALIZED_SIZE: usize = 14; +// bytemuck requires structures to be aligned +pub const SIGNATURE_OFFSETS_START: usize = 2; +pub const DATA_START: usize = SIGNATURE_OFFSETS_SERIALIZED_SIZE + SIGNATURE_OFFSETS_START; + #[derive(Accounts)] pub struct CopyGossipContactInfo<'info> { #[account( @@ -22,15 +44,21 @@ pub struct CopyGossipContactInfo<'info> { /// CHECK: Safe because it's a sysvar account #[account(address = sysvar::instructions::ID)] pub instructions: UncheckedAccount<'info>, + #[account( + seeds = [Config::SEED], + bump = config.bump, + has_one = oracle_authority + )] + pub config: Account<'info, Config>, #[account(mut)] - pub signer: Signer<'info>, + pub oracle_authority: Signer<'info>, } pub fn handle_copy_gossip_contact_info(ctx: Context) -> Result<()> { let mut validator_history_account = ctx.accounts.validator_history_account.load_mut()?; let instructions = ctx.accounts.instructions.to_account_info(); let clock = Clock::get()?; - let epoch = cast_epoch(clock.epoch); + let epoch = cast_epoch(clock.epoch)?; let verify_instruction = sysvar::instructions::get_instruction_relative(-1, &instructions)?; @@ -39,9 +67,16 @@ pub fn handle_copy_gossip_contact_info(ctx: Context) -> R return Err(ValidatorHistoryError::NotSigVerified.into()); } - let message_signer = Pubkey::try_from(&verify_instruction.data[16..48]) - .map_err(|_| ValidatorHistoryError::GossipDataInvalid)?; - let message_data = &verify_instruction.data[112..]; + let ed25519_offsets = from_bytes::( + &verify_instruction.data[SIGNATURE_OFFSETS_START..SIGNATURE_OFFSETS_SERIALIZED_SIZE], + ); + + let message_signer = Pubkey::try_from( + &verify_instruction.data[ed25519_offsets.public_key_offset as usize + ..ed25519_offsets.public_key_offset as usize + PUBKEY_SERIALIZED_SIZE], + ) + .map_err(|_| ValidatorHistoryError::GossipDataInvalid)?; + let message_data = &verify_instruction.data[ed25519_offsets.message_data_offset as usize..]; let crds_data: CrdsData = bincode::deserialize(message_data).map_err(|_| ValidatorHistoryError::GossipDataInvalid)?; diff --git a/programs/validator-history/src/instructions/copy_tip_distribution_account.rs b/programs/validator-history/src/instructions/copy_tip_distribution_account.rs index e69c8fc5..4862249f 100644 --- a/programs/validator-history/src/instructions/copy_tip_distribution_account.rs +++ b/programs/validator-history/src/instructions/copy_tip_distribution_account.rs @@ -57,7 +57,7 @@ pub fn handle_copy_tip_distribution_account( if epoch > Clock::get()?.epoch { return Err(ValidatorHistoryError::EpochOutOfRange.into()); } - let epoch = cast_epoch(epoch); + let epoch = cast_epoch(epoch)?; 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()?; diff --git a/programs/validator-history/src/instructions/copy_vote_account.rs b/programs/validator-history/src/instructions/copy_vote_account.rs index eb922b46..371a1850 100644 --- a/programs/validator-history/src/instructions/copy_vote_account.rs +++ b/programs/validator-history/src/instructions/copy_vote_account.rs @@ -25,7 +25,7 @@ pub struct CopyVoteAccount<'info> { pub fn handle_copy_vote_account(ctx: Context) -> Result<()> { let mut validator_history_account = ctx.accounts.validator_history_account.load_mut()?; let clock = Clock::get()?; - let epoch = cast_epoch(clock.epoch); + let epoch = cast_epoch(clock.epoch)?; let commission = VoteStateVersions::deserialize_commission(&ctx.accounts.vote_account)?; validator_history_account.set_commission_and_slot(epoch, commission, clock.slot)?; diff --git a/programs/validator-history/src/instructions/update_stake_history.rs b/programs/validator-history/src/instructions/update_stake_history.rs index 3bd9049d..b12fe4b0 100644 --- a/programs/validator-history/src/instructions/update_stake_history.rs +++ b/programs/validator-history/src/instructions/update_stake_history.rs @@ -29,6 +29,8 @@ pub struct UpdateStakeHistory<'info> { pub oracle_authority: Signer<'info>, } +// NOTE: If using this instruction to backfill a new validator history account, you must ensure that epochs are added in ascending order. +// This is because new entries cannot be inserted for an epoch that is lower than the last entry's if missed. pub fn handle_update_stake_history( ctx: Context, epoch: u64, @@ -36,13 +38,14 @@ pub fn handle_update_stake_history( rank: u32, is_superminority: bool, ) -> Result<()> { - let mut validator_history_account = ctx.accounts.validator_history_account.load_mut()?; + let mut validator_history_account: std::cell::RefMut<'_, ValidatorHistory> = + ctx.accounts.validator_history_account.load_mut()?; // Cannot set stake for future epochs if epoch > Clock::get()?.epoch { return Err(ValidatorHistoryError::EpochOutOfRange.into()); } - let epoch = cast_epoch(epoch); + let epoch = cast_epoch(epoch)?; validator_history_account.set_stake(epoch, lamports, rank, is_superminority)?; diff --git a/programs/validator-history/src/state.rs b/programs/validator-history/src/state.rs index 0ae26d00..87748aa3 100644 --- a/programs/validator-history/src/state.rs +++ b/programs/validator-history/src/state.rs @@ -61,11 +61,13 @@ pub struct ValidatorHistoryEntry { pub rank: u32, // Most recent updated slot for epoch credits and commission pub vote_account_last_update_slot: u64, - // MEV earned, stored as 1/100th SOL. mev_earned = 100 means 1 SOL earned + // MEV earned, stored as 1/100th SOL. mev_earned = 100 means 1.00 SOL earned pub mev_earned: u32, pub padding1: [u8; 84], } +// Default values for fields in `ValidatorHistoryEntry` are the type's max value. +// It's important to ensure that the max value is not a valid value for the field, so we can check if the field has been set. impl Default for ValidatorHistoryEntry { fn default() -> Self { Self { @@ -378,7 +380,7 @@ impl ValidatorHistory { let epoch_credits_map: HashMap = HashMap::from_iter(epoch_credits.iter().map(|(epoch, cur, prev)| { ( - cast_epoch(*epoch), + cast_epoch(*epoch).unwrap(), // all epochs in list will be valid if current epoch is valid (cur.checked_sub(*prev) .ok_or(ValidatorHistoryError::InvalidEpochCredits) .unwrap() as u32), @@ -654,9 +656,10 @@ pub struct ClusterHistory { #[zero_copy] pub struct ClusterHistoryEntry { pub total_blocks: u32, - pub epoch_start_timestamp: u32, pub epoch: u16, - pub padding: [u8; 246], + pub padding0: [u8; 2], + pub epoch_start_timestamp: u64, + pub padding: [u8; 240], } impl Default for ClusterHistoryEntry { @@ -664,8 +667,9 @@ impl Default for ClusterHistoryEntry { Self { total_blocks: u32::MAX, epoch: u16::MAX, - epoch_start_timestamp: u32::MAX, - padding: [u8::MAX; 246], + padding0: [u8::MAX; 2], + epoch_start_timestamp: u64::MAX, + padding: [u8::MAX; 240], } } } @@ -798,7 +802,7 @@ impl ClusterHistory { pub fn set_epoch_start_timestamp( &mut self, epoch: u16, - epoch_start_timestamp: u32, + epoch_start_timestamp: u64, ) -> Result<()> { if let Some(entry) = self.history.last_mut() { if entry.epoch == epoch { diff --git a/programs/validator-history/src/utils.rs b/programs/validator-history/src/utils.rs index a4bc34fc..13c44da1 100644 --- a/programs/validator-history/src/utils.rs +++ b/programs/validator-history/src/utils.rs @@ -1,13 +1,21 @@ use anchor_lang::{ - prelude::{AccountInfo, Pubkey}, + prelude::{AccountInfo, Pubkey, Result}, + require, solana_program::native_token::lamports_to_sol, }; -pub fn cast_epoch(epoch: u64) -> u16 { - (epoch % u16::MAX as u64).try_into().unwrap() +use crate::errors::ValidatorHistoryError; + +pub fn cast_epoch(epoch: u64) -> Result { + require!( + epoch < (u16::MAX as u64), + ValidatorHistoryError::EpochTooLarge + ); + let epoch_u16: u16 = (epoch % u16::MAX as u64).try_into().unwrap(); + Ok(epoch_u16) } -pub fn cast_epoch_start_timestamp(start_timestamp: i64) -> u32 { +pub fn cast_epoch_start_timestamp(start_timestamp: i64) -> u64 { start_timestamp.try_into().unwrap() } diff --git a/tests/tests/test_cluster_history.rs b/tests/tests/test_cluster_history.rs index e696357d..7929caf0 100644 --- a/tests/tests/test_cluster_history.rs +++ b/tests/tests/test_cluster_history.rs @@ -113,10 +113,10 @@ async fn test_start_epoch_timestamp() { assert_eq!(account.history.arr[0].epoch, 0); assert_eq!(account.history.arr[1].epoch, 1); - assert_ne!(account.history.arr[0].epoch_start_timestamp, u32::MAX); - assert_ne!(account.history.arr[1].epoch_start_timestamp, u32::MAX); + assert_ne!(account.history.arr[0].epoch_start_timestamp, u64::MAX); + assert_ne!(account.history.arr[1].epoch_start_timestamp, u64::MAX); assert_eq!( account.history.arr[0].epoch_start_timestamp, - account.history.arr[1].epoch_start_timestamp - (dif_slots * MS_PER_SLOT) as u32 + account.history.arr[1].epoch_start_timestamp - dif_slots * MS_PER_SLOT ); } diff --git a/tests/tests/test_gossip.rs b/tests/tests/test_gossip.rs index 2a9eb92b..44fa85ff 100644 --- a/tests/tests/test_gossip.rs +++ b/tests/tests/test_gossip.rs @@ -10,7 +10,7 @@ use solana_gossip::{ }; use solana_program_test::*; use solana_sdk::{ - clock::Clock, ed25519_instruction::new_ed25519_instruction, signer::Signer, + clock::Clock, ed25519_instruction::new_ed25519_instruction, pubkey::Pubkey, signer::Signer, transaction::Transaction, }; use solana_version::LegacyVersion2; @@ -35,7 +35,8 @@ fn create_gossip_tx(fixture: &TestFixture, crds_data: &CrdsData) -> Transaction validator_history_account: fixture.validator_history_account, vote_account: fixture.vote_account, instructions: anchor_lang::solana_program::sysvar::instructions::id(), - signer: fixture.keypair.pubkey(), + config: fixture.validator_history_config, + oracle_authority: fixture.keypair.pubkey(), } .to_account_metas(None), data: validator_history::instruction::CopyGossipContactInfo {}.data(), @@ -140,7 +141,8 @@ async fn test_copy_legacy_version() { validator_history_account: fixture.validator_history_account, vote_account: fixture.vote_account, instructions: anchor_lang::solana_program::sysvar::instructions::id(), - signer: fixture.keypair.pubkey(), + config: fixture.validator_history_config, + oracle_authority: fixture.keypair.pubkey(), } .to_account_metas(None), data: validator_history::instruction::CopyGossipContactInfo {}.data(), @@ -224,7 +226,8 @@ async fn test_gossip_wrong_signer() { validator_history_account: fixture.validator_history_account, vote_account: fixture.vote_account, instructions: anchor_lang::solana_program::sysvar::instructions::id(), - signer: fixture.keypair.pubkey(), + config: fixture.validator_history_config, + oracle_authority: fixture.keypair.pubkey(), } .to_account_metas(None), data: validator_history::instruction::CopyGossipContactInfo {}.data(), @@ -281,7 +284,8 @@ async fn test_gossip_missing_sigverify_instruction() { validator_history_account: fixture.validator_history_account, vote_account: fixture.vote_account, instructions: anchor_lang::solana_program::sysvar::instructions::id(), - signer: fixture.keypair.pubkey(), + config: fixture.validator_history_config, + oracle_authority: fixture.keypair.pubkey(), } .to_account_metas(None), data: validator_history::instruction::CopyGossipContactInfo {}.data(), diff --git a/utils/validator-history-cli/Cargo.toml b/utils/validator-history-cli/Cargo.toml index 1fbc40e1..8c1be182 100644 --- a/utils/validator-history-cli/Cargo.toml +++ b/utils/validator-history-cli/Cargo.toml @@ -19,4 +19,4 @@ solana-program = "1.16" solana-sdk = "1.16" thiserror = "1.0.37" tokio = { version = "1.36.0", features = ["full"] } -validator-history = { path = "../../programs/validator-history" } +validator-history = { path = "../../programs/validator-history", features = ["no-entrypoint"]} From 080e647129854e1d8a50fb7705eb47508c3e6bec Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 13:36:03 -0500 Subject: [PATCH 02/10] clippylint --- .../idl/validator_history.json | 33 +++++++++++++++---- tests/tests/test_gossip.rs | 2 +- utils/validator-history-cli/Cargo.toml | 2 +- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/programs/validator-history/idl/validator_history.json b/programs/validator-history/idl/validator_history.json index b06cfd67..f36c0afe 100644 --- a/programs/validator-history/idl/validator_history.json +++ b/programs/validator-history/idl/validator_history.json @@ -318,7 +318,12 @@ "isSigner": false }, { - "name": "signer", + "name": "config", + "isMut": false, + "isSigner": false + }, + { + "name": "oracleAuthority", "isMut": true, "isSigner": true } @@ -649,20 +654,29 @@ "name": "totalBlocks", "type": "u32" }, - { - "name": "epochStartTimestamp", - "type": "u32" - }, { "name": "epoch", "type": "u16" }, + { + "name": "padding0", + "type": { + "array": [ + "u8", + 2 + ] + } + }, + { + "name": "epochStartTimestamp", + "type": "u64" + }, { "name": "padding", "type": { "array": [ "u8", - 246 + 240 ] } } @@ -801,7 +815,7 @@ "type": "u8" }, { - "name": "numAddrs", + "name": "num_addrs", "type": { "defined": "usize" } @@ -935,6 +949,11 @@ "code": 6010, "name": "SlotHistoryOutOfDate", "msg": "Slot history sysvar is not containing expected slots" + }, + { + "code": 6011, + "name": "EpochTooLarge", + "msg": "Epoch larger than 65535, cannot be stored" } ] } \ No newline at end of file diff --git a/tests/tests/test_gossip.rs b/tests/tests/test_gossip.rs index 44fa85ff..713cc6d2 100644 --- a/tests/tests/test_gossip.rs +++ b/tests/tests/test_gossip.rs @@ -10,7 +10,7 @@ use solana_gossip::{ }; use solana_program_test::*; use solana_sdk::{ - clock::Clock, ed25519_instruction::new_ed25519_instruction, pubkey::Pubkey, signer::Signer, + clock::Clock, ed25519_instruction::new_ed25519_instruction, signer::Signer, transaction::Transaction, }; use solana_version::LegacyVersion2; diff --git a/utils/validator-history-cli/Cargo.toml b/utils/validator-history-cli/Cargo.toml index 8c1be182..ed067e11 100644 --- a/utils/validator-history-cli/Cargo.toml +++ b/utils/validator-history-cli/Cargo.toml @@ -19,4 +19,4 @@ solana-program = "1.16" solana-sdk = "1.16" thiserror = "1.0.37" tokio = { version = "1.36.0", features = ["full"] } -validator-history = { path = "../../programs/validator-history", features = ["no-entrypoint"]} +validator-history = { features = ["no-entrypoint"], path = "../../programs/validator-history" } From fbc9330fef7814eb2ead6ff28de060a90f977d31 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 13:41:50 -0500 Subject: [PATCH 03/10] .. --- .../idl/validator_history.json | 74 +++++-------------- 1 file changed, 17 insertions(+), 57 deletions(-) diff --git a/programs/validator-history/idl/validator_history.json b/programs/validator-history/idl/validator_history.json index f36c0afe..6e4a3676 100644 --- a/programs/validator-history/idl/validator_history.json +++ b/programs/validator-history/idl/validator_history.json @@ -45,9 +45,7 @@ "name": "voteAccount", "isMut": false, "isSigner": false, - "docs": [ - "Used to read validator commission." - ] + "docs": ["Used to read validator commission."] }, { "name": "systemProgram", @@ -137,9 +135,7 @@ "name": "voteAccount", "isMut": false, "isSigner": false, - "docs": [ - "Used to read validator commission." - ] + "docs": ["Used to read validator commission."] }, { "name": "config", @@ -435,10 +431,7 @@ { "name": "padding0", "type": { - "array": [ - "u8", - 7 - ] + "array": ["u8", 7] } }, { @@ -452,10 +445,7 @@ { "name": "padding1", "type": { - "array": [ - "u8", - 232 - ] + "array": ["u8", 232] } }, { @@ -483,10 +473,7 @@ { "name": "padding0", "type": { - "array": [ - "u8", - 7 - ] + "array": ["u8", 7] } }, { @@ -496,10 +483,7 @@ { "name": "padding1", "type": { - "array": [ - "u8", - 232 - ] + "array": ["u8", 232] } }, { @@ -551,10 +535,7 @@ { "name": "ip", "type": { - "array": [ - "u8", - 4 - ] + "array": ["u8", 4] } }, { @@ -580,10 +561,7 @@ { "name": "padding1", "type": { - "array": [ - "u8", - 84 - ] + "array": ["u8", 84] } } ] @@ -625,10 +603,7 @@ { "name": "padding", "type": { - "array": [ - "u8", - 7 - ] + "array": ["u8", 7] } }, { @@ -661,10 +636,7 @@ { "name": "padding0", "type": { - "array": [ - "u8", - 2 - ] + "array": ["u8", 2] } }, { @@ -674,10 +646,7 @@ { "name": "padding", "type": { - "array": [ - "u8", - 240 - ] + "array": ["u8", 240] } } ] @@ -699,10 +668,7 @@ { "name": "padding", "type": { - "array": [ - "u8", - 7 - ] + "array": ["u8", 7] } }, { @@ -803,9 +769,7 @@ }, { "name": "DuplicateSocket", - "fields": [ - "u8" - ] + "fields": ["u8"] }, { "name": "InvalidIpAddrIndex", @@ -815,7 +779,7 @@ "type": "u8" }, { - "name": "num_addrs", + "name": "", "type": { "defined": "usize" } @@ -824,9 +788,7 @@ }, { "name": "InvalidPort", - "fields": [ - "u16" - ] + "fields": ["u16"] }, { "name": "InvalidQuicSocket", @@ -859,9 +821,7 @@ }, { "name": "SocketNotFound", - "fields": [ - "u8" - ] + "fields": ["u8"] }, { "name": "UnspecifiedIpAddr", @@ -956,4 +916,4 @@ "msg": "Epoch larger than 65535, cannot be stored" } ] -} \ No newline at end of file +} From 34206f2c8d42a717ccfcbdc16ff93610322a0e20 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 13:50:26 -0500 Subject: [PATCH 04/10] .. --- .../idl/validator_history.json | 72 ++++++++++++++----- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/programs/validator-history/idl/validator_history.json b/programs/validator-history/idl/validator_history.json index 6e4a3676..03155370 100644 --- a/programs/validator-history/idl/validator_history.json +++ b/programs/validator-history/idl/validator_history.json @@ -45,7 +45,9 @@ "name": "voteAccount", "isMut": false, "isSigner": false, - "docs": ["Used to read validator commission."] + "docs": [ + "Used to read validator commission." + ] }, { "name": "systemProgram", @@ -135,7 +137,9 @@ "name": "voteAccount", "isMut": false, "isSigner": false, - "docs": ["Used to read validator commission."] + "docs": [ + "Used to read validator commission." + ] }, { "name": "config", @@ -431,7 +435,10 @@ { "name": "padding0", "type": { - "array": ["u8", 7] + "array": [ + "u8", + 7 + ] } }, { @@ -445,7 +452,10 @@ { "name": "padding1", "type": { - "array": ["u8", 232] + "array": [ + "u8", + 232 + ] } }, { @@ -473,7 +483,10 @@ { "name": "padding0", "type": { - "array": ["u8", 7] + "array": [ + "u8", + 7 + ] } }, { @@ -483,7 +496,10 @@ { "name": "padding1", "type": { - "array": ["u8", 232] + "array": [ + "u8", + 232 + ] } }, { @@ -535,7 +551,10 @@ { "name": "ip", "type": { - "array": ["u8", 4] + "array": [ + "u8", + 4 + ] } }, { @@ -561,7 +580,10 @@ { "name": "padding1", "type": { - "array": ["u8", 84] + "array": [ + "u8", + 84 + ] } } ] @@ -603,7 +625,10 @@ { "name": "padding", "type": { - "array": ["u8", 7] + "array": [ + "u8", + 7 + ] } }, { @@ -636,7 +661,10 @@ { "name": "padding0", "type": { - "array": ["u8", 2] + "array": [ + "u8", + 2 + ] } }, { @@ -646,7 +674,10 @@ { "name": "padding", "type": { - "array": ["u8", 240] + "array": [ + "u8", + 240 + ] } } ] @@ -668,7 +699,10 @@ { "name": "padding", "type": { - "array": ["u8", 7] + "array": [ + "u8", + 7 + ] } }, { @@ -769,7 +803,9 @@ }, { "name": "DuplicateSocket", - "fields": ["u8"] + "fields": [ + "u8" + ] }, { "name": "InvalidIpAddrIndex", @@ -779,7 +815,7 @@ "type": "u8" }, { - "name": "", + "name": "numAddrs", "type": { "defined": "usize" } @@ -788,7 +824,9 @@ }, { "name": "InvalidPort", - "fields": ["u16"] + "fields": [ + "u16" + ] }, { "name": "InvalidQuicSocket", @@ -821,7 +859,9 @@ }, { "name": "SocketNotFound", - "fields": ["u8"] + "fields": [ + "u8" + ] }, { "name": "UnspecifiedIpAddr", From 2999b2eca5bacfd317df88da0596c859562c711e Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 13:56:47 -0500 Subject: [PATCH 05/10] update mio --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 95e0cefb..6abbfdc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2701,9 +2701,9 @@ dependencies = [ [[package]] name = "mio" -version = "0.8.10" +version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f3d0b296e374a4e6f3c7b0a1f5a51d748a0d34c85e7dc48fc3fa9a87657fe09" +checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" dependencies = [ "libc", "wasi 0.11.0+wasi-snapshot-preview1", From c2dce1c60d800414846c4a20417aed17bee67e31 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 14:05:44 -0500 Subject: [PATCH 06/10] idl --- programs/validator-history/idl/validator_history.json | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/validator-history/idl/validator_history.json b/programs/validator-history/idl/validator_history.json index 03155370..360b353c 100644 --- a/programs/validator-history/idl/validator_history.json +++ b/programs/validator-history/idl/validator_history.json @@ -957,3 +957,4 @@ } ] } + From e01229e5d772a7cf57f70e2395749b01ec036bd9 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 14:24:36 -0500 Subject: [PATCH 07/10] ? --- programs/validator-history/idl/validator_history.json | 1 - 1 file changed, 1 deletion(-) diff --git a/programs/validator-history/idl/validator_history.json b/programs/validator-history/idl/validator_history.json index 360b353c..03155370 100644 --- a/programs/validator-history/idl/validator_history.json +++ b/programs/validator-history/idl/validator_history.json @@ -957,4 +957,3 @@ } ] } - From cfd218ca029946f63afc547e65458b50e4e78171 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 14:31:25 -0500 Subject: [PATCH 08/10] finally --- programs/validator-history/idl/validator_history.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/validator-history/idl/validator_history.json b/programs/validator-history/idl/validator_history.json index 03155370..82d5f431 100644 --- a/programs/validator-history/idl/validator_history.json +++ b/programs/validator-history/idl/validator_history.json @@ -956,4 +956,4 @@ "msg": "Epoch larger than 65535, cannot be stored" } ] -} +} \ No newline at end of file From 20b28e86f474571544029dc96062d9a5ae9f893e Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 15:01:17 -0500 Subject: [PATCH 09/10] fix indexing math --- .../src/instructions/copy_gossip_contact_info.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/programs/validator-history/src/instructions/copy_gossip_contact_info.rs b/programs/validator-history/src/instructions/copy_gossip_contact_info.rs index 2ea906ea..ffdf1040 100644 --- a/programs/validator-history/src/instructions/copy_gossip_contact_info.rs +++ b/programs/validator-history/src/instructions/copy_gossip_contact_info.rs @@ -68,7 +68,8 @@ pub fn handle_copy_gossip_contact_info(ctx: Context) -> R } let ed25519_offsets = from_bytes::( - &verify_instruction.data[SIGNATURE_OFFSETS_START..SIGNATURE_OFFSETS_SERIALIZED_SIZE], + &verify_instruction.data + [SIGNATURE_OFFSETS_START..SIGNATURE_OFFSETS_START + SIGNATURE_OFFSETS_SERIALIZED_SIZE], ); let message_signer = Pubkey::try_from( From 76723a332bb0e49546c074ddbec6bdee9e2a0921 Mon Sep 17 00:00:00 2001 From: Evan Batsell Date: Mon, 4 Mar 2024 18:14:00 -0500 Subject: [PATCH 10/10] add test for ed25519 check --- .../instructions/copy_gossip_contact_info.rs | 27 +++- tests/tests/test_gossip.rs | 124 +++++++++++++++++- 2 files changed, 142 insertions(+), 9 deletions(-) diff --git a/programs/validator-history/src/instructions/copy_gossip_contact_info.rs b/programs/validator-history/src/instructions/copy_gossip_contact_info.rs index ffdf1040..218a8bfa 100644 --- a/programs/validator-history/src/instructions/copy_gossip_contact_info.rs +++ b/programs/validator-history/src/instructions/copy_gossip_contact_info.rs @@ -14,13 +14,13 @@ use validator_history_vote_state::VoteStateVersions; #[derive(Default, Debug, Copy, Clone, Zeroable, Pod, Eq, PartialEq)] #[repr(C)] pub struct Ed25519SignatureOffsets { - signature_offset: u16, // offset to ed25519 signature of 64 bytes - signature_instruction_index: u16, // instruction index to find signature - pub public_key_offset: u16, // offset to public key of 32 bytes - public_key_instruction_index: u16, // instruction index to find public key - pub message_data_offset: u16, // offset to start of message data - message_data_size: u16, // size of message data - message_instruction_index: u16, // index of instruction data to get message data + pub signature_offset: u16, // offset to ed25519 signature of 64 bytes + pub signature_instruction_index: u16, // instruction index to find signature + pub public_key_offset: u16, // offset to public key of 32 bytes + pub public_key_instruction_index: u16, // instruction index to find public key + pub message_data_offset: u16, // offset to start of message data + pub message_data_size: u16, // size of message data + pub message_instruction_index: u16, // index of instruction data to get message data } pub const PUBKEY_SERIALIZED_SIZE: usize = 32; @@ -72,6 +72,19 @@ pub fn handle_copy_gossip_contact_info(ctx: Context) -> R [SIGNATURE_OFFSETS_START..SIGNATURE_OFFSETS_START + SIGNATURE_OFFSETS_SERIALIZED_SIZE], ); + // Check offsets and indices are correct so an attacker cannot submit invalid data + if ed25519_offsets.signature_instruction_index != ed25519_offsets.public_key_instruction_index + || ed25519_offsets.signature_instruction_index != ed25519_offsets.message_instruction_index + || ed25519_offsets.public_key_offset + != (SIGNATURE_OFFSETS_START + SIGNATURE_OFFSETS_SERIALIZED_SIZE) as u16 + || ed25519_offsets.signature_offset + != ed25519_offsets.public_key_offset + PUBKEY_SERIALIZED_SIZE as u16 + || ed25519_offsets.message_data_offset + != ed25519_offsets.signature_offset + SIGNATURE_SERIALIZED_SIZE as u16 + { + return Err(ValidatorHistoryError::GossipDataInvalid.into()); + } + let message_signer = Pubkey::try_from( &verify_instruction.data[ed25519_offsets.public_key_offset as usize ..ed25519_offsets.public_key_offset as usize + PUBKEY_SERIALIZED_SIZE], diff --git a/tests/tests/test_gossip.rs b/tests/tests/test_gossip.rs index 713cc6d2..78fb5be9 100644 --- a/tests/tests/test_gossip.rs +++ b/tests/tests/test_gossip.rs @@ -2,6 +2,8 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use anchor_lang::{solana_program::instruction::Instruction, InstructionData, ToAccountMetas}; use bincode::serialize; +use bytemuck::bytes_of; +use ed25519_dalek::Signer as Ed25519Signer; use rand::thread_rng; use solana_gossip::{ contact_info::ContactInfo, @@ -10,14 +12,18 @@ use solana_gossip::{ }; use solana_program_test::*; use solana_sdk::{ - clock::Clock, ed25519_instruction::new_ed25519_instruction, signer::Signer, + clock::Clock, + ed25519_instruction::{ + new_ed25519_instruction, DATA_START, PUBKEY_SERIALIZED_SIZE, SIGNATURE_SERIALIZED_SIZE, + }, + signer::Signer, transaction::Transaction, }; use solana_version::LegacyVersion2; use tests::fixtures::TestFixture; use validator_history::{ crds_value::{CrdsData as ValidatorHistoryCrdsData, LegacyVersion, LegacyVersion1}, - ValidatorHistory, + Ed25519SignatureOffsets, ValidatorHistory, }; fn create_gossip_tx(fixture: &TestFixture, crds_data: &CrdsData) -> Transaction { @@ -407,3 +413,117 @@ async fn test_gossip_timestamps() { .submit_transaction_assert_error(transaction, "GossipDataInFuture") .await; } + +#[tokio::test] +async fn test_fake_offsets() { + // Put in fake offsets, and make sure we get a GossipDataInvalid error + let fixture = TestFixture::new().await; + fixture.initialize_config().await; + fixture.initialize_validator_history_account().await; + let mut banks_client = { + let ctx = fixture.ctx.borrow_mut(); + ctx.banks_client.clone() + }; + + // Initial valid instruction + let dalek_keypair = + ed25519_dalek::Keypair::from_bytes(&fixture.identity_keypair.to_bytes()).unwrap(); + + let clock: Clock = banks_client.get_sysvar().await.unwrap(); + let wallclock = clock.unix_timestamp as u64 * 1000; + let mut contact_info = ContactInfo::new(fixture.identity_keypair.pubkey(), wallclock, 0); + let ipv4 = Ipv4Addr::new(1, 2, 3, 4); + let ip = IpAddr::V4(ipv4); + contact_info + .set_socket(0, SocketAddr::new(ip, 1234)) + .expect("could not set socket"); + let crds_data = CrdsData::ContactInfo(contact_info.clone()); + + let ed25519_ix = new_ed25519_instruction(&dalek_keypair, &serialize(&crds_data).unwrap()); + + // Invalid instruction + let fake_ipv4 = Ipv4Addr::new(5, 5, 5, 5); + let fake_ip = IpAddr::V4(fake_ipv4); + let mut contact_info = ContactInfo::new(fixture.identity_keypair.pubkey(), wallclock, 0); + contact_info + .set_socket(0, SocketAddr::new(fake_ip, 1234)) + .unwrap(); + let crds_data = CrdsData::ContactInfo(contact_info.clone()); + + // Code from new_ed25519_instruction with modified instruction indices + let message = serialize(&crds_data).unwrap(); + + let signature = dalek_keypair.sign(&message).to_bytes(); + let pubkey = dalek_keypair.public.to_bytes(); + + assert_eq!(pubkey.len(), PUBKEY_SERIALIZED_SIZE); + assert_eq!(signature.len(), SIGNATURE_SERIALIZED_SIZE); + + let mut instruction_data = Vec::with_capacity( + DATA_START + .saturating_add(SIGNATURE_SERIALIZED_SIZE) + .saturating_add(PUBKEY_SERIALIZED_SIZE) + .saturating_add(message.len()), + ); + + let num_signatures: u8 = 1; + let public_key_offset = DATA_START; + let signature_offset = public_key_offset.saturating_add(PUBKEY_SERIALIZED_SIZE); + let message_data_offset = signature_offset.saturating_add(SIGNATURE_SERIALIZED_SIZE); + + instruction_data.extend_from_slice(bytes_of(&[num_signatures, 0])); + + let offsets = Ed25519SignatureOffsets { + signature_offset: signature_offset as u16, + signature_instruction_index: 0, // Index of real signature + public_key_offset: public_key_offset as u16, + public_key_instruction_index: 0, // Index of real signer + message_data_offset: message_data_offset as u16, + message_data_size: message.len() as u16, + message_instruction_index: 1, // Index of fake data + }; + + instruction_data.extend_from_slice(bytes_of(&offsets)); + + debug_assert_eq!(instruction_data.len(), public_key_offset); + + instruction_data.extend_from_slice(&pubkey); + + debug_assert_eq!(instruction_data.len(), signature_offset); + + instruction_data.extend_from_slice(&signature); + + debug_assert_eq!(instruction_data.len(), message_data_offset); + + instruction_data.extend_from_slice(&message); + + let fake_instruction = Instruction { + program_id: solana_sdk::ed25519_program::id(), + accounts: vec![], + data: instruction_data, + }; + + let copy_gossip_ix = Instruction { + program_id: validator_history::id(), + accounts: validator_history::accounts::CopyGossipContactInfo { + validator_history_account: fixture.validator_history_account, + vote_account: fixture.vote_account, + instructions: anchor_lang::solana_program::sysvar::instructions::id(), + config: fixture.validator_history_config, + oracle_authority: fixture.keypair.pubkey(), + } + .to_account_metas(None), + data: validator_history::instruction::CopyGossipContactInfo {}.data(), + }; + + let transaction = Transaction::new_signed_with_payer( + &[ed25519_ix, fake_instruction, copy_gossip_ix], + Some(&fixture.keypair.pubkey()), + &[&fixture.keypair], + fixture.ctx.borrow().last_blockhash, + ); + + fixture + .submit_transaction_assert_error(transaction, "GossipDataInvalid") + .await; +}