gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

uses enum instead of trait for VoteTransaction (#22019)

Browse Source 
Box<dyn Trait> involves runtime dispatch, has significant overhead and
is slow. It also requires hacky boilerplate code for implementing Clone
or other basic traits:
https://github.com/solana-labs/solana/blob/e92a81b74/programs/vote/src/vote_state/mod.rs#L70-L102

Only limited known types can be VoteTransaction and they are all defined
in the same crate. So using a trait here only adds overhead.
https://github.com/solana-labs/solana/blob/e92a81b74/programs/vote/src/vote_state/mod.rs#L125-L165
https://github.com/solana-labs/solana/blob/e92a81b74/programs/vote/src/vote_state/mod.rs#L221-L264
This commit is contained in:
behzad nouri
2021-12-22 14:25:46 +00:00
committed by GitHub
parent d6de4a2f4e
commit 4d62f03297
10 changed files with 118 additions and 325 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
programs/bpf/Cargo.lock generated
View File

@ -600,16 +600,6 @@ dependencies = [
"subtle",
]
[[package]]
name = "ctor"
version = "0.1.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7fbaabec2c953050352311293be5c6aba8e141ba19d6811862b232d6fd020484"
dependencies = [
"quote 1.0.6",
"syn 1.0.67",
]
[[package]]
name = "curve25519-dalek"
version = "2.1.0"
@ -880,15 +870,6 @@ dependencies = [
"termcolor",
]
[[package]]
name = "erased-serde"
version = "0.3.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3de9ad4541d99dc22b59134e7ff8dc3d6c988c89ecd7324bf10a8362b07a2afa"
dependencies = [
"serde",
]
[[package]]
name = "errno"
version = "0.2.8"
@ -1126,17 +1107,6 @@ dependencies = [
"wasi 0.10.1+wasi-snapshot-preview1",
]
[[package]]
name = "ghost"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1a5bcf1bbeab73aa4cf2fde60a846858dc036163c7c33bec309f8d17de785479"
dependencies = [
"proc-macro2 1.0.24",
"quote 1.0.6",
"syn 1.0.67",
]
[[package]]
name = "gimli"
version = "0.21.0"
@ -1383,16 +1353,6 @@ dependencies = [
"cfg-if 1.0.0",
]
[[package]]
name = "inventory"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1367fed6750ff2a5bcb967a631528303bb85631f167a75eb1bf7762d57eb7678"
dependencies = [
"ctor",
"ghost",
]
[[package]]
name = "ipnet"
version = "2.3.0"
@ -3610,7 +3570,6 @@ dependencies = [
"solana-program-runtime",
"solana-sdk",
"thiserror",
"typetag",
]
[[package]]
@ -4084,30 +4043,6 @@ version = "1.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "373c8a200f9e67a0c95e62a4f52fbf80c23b4381c05a17845531982fa99e6b33"
[[package]]
name = "typetag"
version = "0.1.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4080564c5b2241b5bff53ab610082234e0c57b0417f4bd10596f183001505b8a"
dependencies = [
"erased-serde",
"inventory",
"once_cell",
"serde",
"typetag-impl",
]
[[package]]
name = "typetag-impl"
version = "0.1.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e60147782cc30833c05fba3bab1d9b5771b2685a2557672ac96fa5d154099c0e"
dependencies = [
"proc-macro2 1.0.24",
"quote 1.0.6",
"syn 1.0.67",
]
[[package]]
name = "unicode-bidi"
version = "0.3.4"

1
programs/vote/Cargo.toml
View File

@ -22,7 +22,6 @@ solana-logger = { path = "../../logger", version = "=1.10.0" }
solana-metrics = { path = "../../metrics", version = "=1.10.0" }
solana-program-runtime = { path = "../../program-runtime", version = "=1.10.0" }
solana-sdk = { path = "../../sdk", version = "=1.10.0" }
typetag = "0.1"
thiserror = "1.0"
[build-dependencies]

222
programs/vote/src/vote_state/mod.rs
View File

@ -19,7 +19,6 @@ use {
sysvar::clock::Clock,
},
std::{
any::Any,
boxed::Box,
cmp::Ordering,
collections::{HashSet, VecDeque},
@ -41,63 +40,70 @@ pub const MAX_EPOCH_CREDITS_HISTORY: usize = 64;
// Offset of VoteState::prior_voters, for determining initialization status without deserialization
const DEFAULT_PRIOR_VOTERS_OFFSET: usize = 82;
// VoteTransactionClone hack is done so that we can derive clone on the tower that uses the
// VoteTransaction trait object. Clone doesn't work here because it returns Self which is not
// allowed for trait objects
#[typetag::serde{tag = "type"}]
pub trait VoteTransaction: VoteTransactionClone + Debug + Send {
fn slot(&self, i: usize) -> Slot;
fn len(&self) -> usize;
fn hash(&self) -> Hash;
fn timestamp(&self) -> Option<UnixTimestamp>;
fn last_voted_slot(&self) -> Option<Slot>;
fn last_voted_slot_hash(&self) -> Option<(Slot, Hash)>;
fn set_timestamp(&mut self, ts: Option<UnixTimestamp>);
fn slots(&self) -> Vec<Slot> {
(0..self.len()).map(|i| self.slot(i)).collect()
}
fn is_empty(&self) -> bool {
self.len() == 0
}
// Have to manually implement because deriving PartialEq returns Self
fn eq(&self, other: &dyn VoteTransaction) -> bool;
fn as_any(&self) -> &dyn Any;
#[derive(Debug, PartialEq)]
pub enum VoteTransaction {
Vote(Vote),
VoteStateUpdate(VoteStateUpdate),
}
pub trait VoteTransactionClone {
fn clone_box(&self) -> Box<dyn VoteTransaction>;
}
impl VoteTransaction {
pub fn slots(&self) -> Vec<Slot> {
match self {
VoteTransaction::Vote(vote) => vote.slots.clone(),
VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update
.lockouts
.iter()
.map(|lockout| lockout.slot)
.collect(),
}
}
impl<T> VoteTransactionClone for T
where
T: VoteTransaction + Clone + 'static,
{
fn clone_box(&self) -> Box<dyn VoteTransaction> {
Box::new(self.clone())
pub fn is_empty(&self) -> bool {
match self {
VoteTransaction::Vote(vote) => vote.slots.is_empty(),
VoteTransaction::VoteStateUpdate(vote_state_update) => {
vote_state_update.lockouts.is_empty()
}
}
}
pub fn hash(&self) -> Hash {
match self {
VoteTransaction::Vote(vote) => vote.hash,
VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.hash,
}
}
pub fn timestamp(&self) -> Option<UnixTimestamp> {
match self {
VoteTransaction::Vote(vote) => vote.timestamp,
VoteTransaction::VoteStateUpdate(vote_state_update) => vote_state_update.timestamp,
}
}
pub fn last_voted_slot(&self) -> Option<Slot> {
match self {
VoteTransaction::Vote(vote) => vote.slots.last().copied(),
VoteTransaction::VoteStateUpdate(vote_state_update) => {
Some(vote_state_update.lockouts.back()?.slot)
}
}
}
pub fn last_voted_slot_hash(&self) -> Option<(Slot, Hash)> {
Some((self.last_voted_slot()?, self.hash()))
}
}
impl Clone for Box<dyn VoteTransaction> {
fn clone(&self) -> Box<dyn VoteTransaction> {
self.clone_box()
impl From<Vote> for VoteTransaction {
fn from(vote: Vote) -> Self {
VoteTransaction::Vote(vote)
}
}
// Have to manually implement because derive returns Self
impl<'a, 'b> PartialEq<dyn VoteTransaction + 'b> for dyn VoteTransaction + 'a {
fn eq(&self, other: &(dyn VoteTransaction + 'b)) -> bool {
VoteTransaction::eq(self, other)
}
}
// This is needed because of weirdness in the derive PartialEq macro
// See rust issue #31740 for more info
impl PartialEq<&Self> for Box<dyn VoteTransaction> {
fn eq(&self, other: &&Self) -> bool {
VoteTransaction::eq(self.as_ref(), other.as_ref())
impl From<VoteStateUpdate> for VoteTransaction {
fn from(vote_state_update: VoteStateUpdate) -> Self {
VoteTransaction::VoteStateUpdate(vote_state_update)
}
}
@ -122,48 +128,6 @@ impl Vote {
}
}
#[typetag::serde]
impl VoteTransaction for Vote {
fn slot(&self, i: usize) -> Slot {
self.slots[i]
}
fn len(&self) -> usize {
self.slots.len()
}
fn hash(&self) -> Hash {
self.hash
}
fn timestamp(&self) -> Option<UnixTimestamp> {
self.timestamp
}
fn last_voted_slot(&self) -> Option<Slot> {
self.slots.last().copied()
}
fn last_voted_slot_hash(&self) -> Option<(Slot, Hash)> {
self.slots.last().copied().map(|slot| (slot, self.hash))
}
fn set_timestamp(&mut self, ts: Option<UnixTimestamp>) {
self.timestamp = ts
}
fn eq(&self, other: &dyn VoteTransaction) -> bool {
other
.as_any()
.downcast_ref::<Self>()
.map_or(false, |x| x == self)
}
fn as_any(&self) -> &dyn Any {
self
}
}
#[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Copy, Clone, AbiExample)]
pub struct Lockout {
pub slot: Slot,
@ -218,51 +182,6 @@ impl VoteStateUpdate {
}
}
#[typetag::serde]
impl VoteTransaction for VoteStateUpdate {
fn slot(&self, i: usize) -> Slot {
self.lockouts[i].slot
}
fn len(&self) -> usize {
self.lockouts.len()
}
fn hash(&self) -> Hash {
self.hash
}
fn timestamp(&self) -> Option<UnixTimestamp> {
self.timestamp
}
fn last_voted_slot(&self) -> Option<Slot> {
self.lockouts.back().copied().map(|lockout| lockout.slot)
}
fn last_voted_slot_hash(&self) -> Option<(Slot, Hash)> {
self.lockouts
.back()
.copied()
.map(|lockout| (lockout.slot, self.hash))
}
fn set_timestamp(&mut self, ts: Option<UnixTimestamp>) {
self.timestamp = ts
}
fn eq(&self, other: &dyn VoteTransaction) -> bool {
other
.as_any()
.downcast_ref::<Self>()
.map_or(false, |x| x == self)
}
fn as_any(&self) -> &dyn Any {
self
}
}
#[derive(Default, Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)]
pub struct VoteInit {
pub node_pubkey: Pubkey,
@ -465,7 +384,7 @@ impl VoteState {
fn check_slots_are_valid(
&self,
vote: &(impl VoteTransaction + Debug),
vote: &Vote,
slot_hashes: &[(Slot, Hash)],
) -> Result<(), VoteError> {
// index into the vote's slots, sarting at the newest
@ -484,19 +403,19 @@ impl VoteState {
//
// 2) Conversely, `slot_hashes` is sorted from newest/largest vote to
// the oldest/smallest vote
while i < vote.len() && j > 0 {
while i < vote.slots.len() && j > 0 {
// 1) increment `i` to find the smallest slot `s` in `vote.slots`
// where `s` >= `last_voted_slot`
if self
.last_voted_slot()
.map_or(false, |last_voted_slot| vote.slot(i) <= last_voted_slot)
.map_or(false, |last_voted_slot| vote.slots[i] <= last_voted_slot)
{
i += 1;
continue;
}
// 2) Find the hash for this slot `s`.
if vote.slot(i) != slot_hashes[j - 1].0 {
if vote.slots[i] != slot_hashes[j - 1].0 {
// Decrement `j` to find newer slots
j -= 1;
continue;
@ -518,7 +437,7 @@ impl VoteState {
);
return Err(VoteError::VoteTooOld);
}
if i != vote.len() {
if i != vote.slots.len() {
// This means there existed some slot for which we couldn't find
// a matching slot hash in step 2)
info!(
@ -528,16 +447,13 @@ impl VoteState {
inc_new_counter_info!("dropped-vote-slot", 1);
return Err(VoteError::SlotsMismatch);
}
if slot_hashes[j].1 != vote.hash() {
if slot_hashes[j].1 != vote.hash {
// This means the newest vote in the slot has a match that
// doesn't match the expected hash for that slot on this
// fork
warn!(
"{} dropped vote {:?} failed to match hash {} {}",
self.node_pubkey,
vote,
vote.hash(),
slot_hashes[j].1
self.node_pubkey, vote, vote.hash, slot_hashes[j].1
);
inc_new_counter_info!("dropped-vote-hash", 1);
return Err(VoteError::SlotHashMismatch);
@ -1160,8 +1076,18 @@ pub fn process_vote_state_update<S: std::hash::BuildHasher>(
signers: &HashSet<Pubkey, S>,
) -> Result<(), InstructionError> {
let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?;
vote_state.check_slots_are_valid(&vote_state_update, slot_hashes)?;
{
let vote = Vote {
slots: vote_state_update
.lockouts
.iter()
.map(|lockout| lockout.slot)
.collect(),
hash: vote_state_update.hash,
timestamp: vote_state_update.timestamp,
};
vote_state.check_slots_are_valid(&vote, slot_hashes)?;
}
vote_state.process_new_vote_state(
vote_state_update.lockouts,
vote_state_update.root,

21
programs/vote/src/vote_transaction.rs
View File

@ -14,22 +14,24 @@ use {
},
};
pub type ParsedVote = (Pubkey, Box<dyn VoteTransaction>, Option<Hash>);
pub type ParsedVote = (Pubkey, VoteTransaction, Option<Hash>);
fn parse_vote(vote_ix: &CompiledInstruction, vote_key: &Pubkey) -> Option<ParsedVote> {
let vote_instruction = limited_deserialize(&vote_ix.data).ok();
vote_instruction.and_then(|vote_instruction| {
let result: Option<ParsedVote> = match vote_instruction {
VoteInstruction::Vote(vote) => Some((*vote_key, Box::new(vote), None)),
VoteInstruction::Vote(vote) => Some((*vote_key, VoteTransaction::from(vote), None)),
VoteInstruction::VoteSwitch(vote, hash) => {
Some((*vote_key, Box::new(vote), Some(hash)))
Some((*vote_key, VoteTransaction::from(vote), Some(hash)))
}
VoteInstruction::UpdateVoteState(vote_state_update) => {
Some((*vote_key, Box::new(vote_state_update), None))
}
VoteInstruction::UpdateVoteStateSwitch(vote_state_update, hash) => {
Some((*vote_key, Box::new(vote_state_update), Some(hash)))
Some((*vote_key, VoteTransaction::from(vote_state_update), None))
}
VoteInstruction::UpdateVoteStateSwitch(vote_state_update, hash) => Some((
*vote_key,
VoteTransaction::from(vote_state_update),
Some(hash),
)),
VoteInstruction::Authorize(_, _)
| VoteInstruction::AuthorizeChecked(_)
| VoteInstruction::InitializeAccount(_)
@ -139,10 +141,7 @@ mod test {
);
let (key, vote, hash) = parse_vote_transaction(&vote_tx).unwrap();
assert_eq!(hash, input_hash);
assert_eq!(
*vote.as_any().downcast_ref::<Vote>().unwrap(),
Vote::new(vec![42], bank_hash)
);
assert_eq!(vote, VoteTransaction::from(Vote::new(vec![42], bank_hash)));
assert_eq!(key, vote_keypair.pubkey());
// Test bad program id fails