Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVM: add new solana-svm-rent-collector crate #2688

Merged
merged 1 commit into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ members = [
"streamer",
"svm",
"svm-conformance",
"svm-rent-collector",
"svm-transaction",
"svm/examples/paytube",
"test-validator",
Expand Down Expand Up @@ -434,6 +435,7 @@ solana-streamer = { path = "streamer", version = "=2.1.0" }
solana-svm = { path = "svm", version = "=2.1.0" }
solana-svm-conformance = { path = "svm-conformance", version = "=2.1.0" }
solana-svm-example-paytube = { path = "svm/examples/paytube", version = "=2.1.0" }
solana-svm-rent-collector = { path = "svm-rent-collector", version = "=2.1.0" }
solana-svm-transaction = { path = "svm-transaction", version = "=2.1.0" }
solana-system-program = { path = "programs/system", version = "=2.1.0" }
solana-test-validator = { path = "test-validator", version = "=2.1.0" }
Expand Down
13 changes: 13 additions & 0 deletions svm-rent-collector/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
name = "solana-svm-rent-collector"
description = "Solana SVM Rent Collector"
documentation = "https://docs.rs/solana-svm-rent-collector"
version = { workspace = true }
authors = { workspace = true }
repository = { workspace = true }
homepage = { workspace = true }
license = { workspace = true }
edition = { workspace = true }

[dependencies]
solana-sdk = { workspace = true }
6 changes: 6 additions & 0 deletions svm-rent-collector/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//! Solana SVM Rent Collector.
//!
//! Rent management for SVM.

pub mod rent_state;
pub mod svm_rent_collector;
15 changes: 15 additions & 0 deletions svm-rent-collector/src/rent_state.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//! Account rent state.

/// Rent state of a Solana account.
#[derive(Debug, PartialEq, Eq)]
pub enum RentState {
/// account.lamports == 0
Uninitialized,
/// 0 < account.lamports < rent-exempt-minimum
RentPaying {
lamports: u64, // account.lamports()
data_size: usize, // account.data().len()
},
/// account.lamports >= rent-exempt-minimum
RentExempt,
}
137 changes: 137 additions & 0 deletions svm-rent-collector/src/svm_rent_collector.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
//! Plugin trait for rent collection within the Solana SVM.

use {
crate::rent_state::RentState,
solana_sdk::{
account::{AccountSharedData, ReadableAccount},
clock::Epoch,
pubkey::Pubkey,
rent::{Rent, RentDue},
rent_collector::CollectedInfo,
transaction::{Result, TransactionError},
transaction_context::{IndexOfAccount, TransactionContext},
},
};

mod rent_collector;

/// Rent collector trait. Represents an entity that can evaluate the rent state
/// of an account, determine rent due, and collect rent.
///
/// Implementors are responsible for evaluating rent due and collecting rent
/// from accounts, if required. Methods for evaluating account rent state have
/// default implementations, which can be overridden for customized rent
/// management.
pub trait SVMRentCollector {
/// Check rent state transition for an account in a transaction.
///
/// This method has a default implementation that calls into
/// `check_rent_state_with_account`.
fn check_rent_state(
&self,
pre_rent_state: Option<&RentState>,
post_rent_state: Option<&RentState>,
transaction_context: &TransactionContext,
index: IndexOfAccount,
) -> Result<()> {
if let Some((pre_rent_state, post_rent_state)) = pre_rent_state.zip(post_rent_state) {
let expect_msg =
"account must exist at TransactionContext index if rent-states are Some";
self.check_rent_state_with_account(
pre_rent_state,
post_rent_state,
transaction_context
.get_key_of_account_at_index(index)
.expect(expect_msg),
&transaction_context
.get_account_at_index(index)
.expect(expect_msg)
.borrow(),
index,
)?;
}
Ok(())
}

/// Check rent state transition for an account directly.
///
/// This method has a default implementation that checks whether the
/// transition is allowed and returns an error if it is not. It also
/// verifies that the account is not the incinerator.
fn check_rent_state_with_account(
&self,
pre_rent_state: &RentState,
post_rent_state: &RentState,
address: &Pubkey,
_account_state: &AccountSharedData,
account_index: IndexOfAccount,
) -> Result<()> {
if !solana_sdk::incinerator::check_id(address)
&& !self.transition_allowed(pre_rent_state, post_rent_state)
{
let account_index = account_index as u8;
Err(TransactionError::InsufficientFundsForRent { account_index })
} else {
Ok(())
}
}
Comment on lines +56 to +77
Copy link
Author

@buffalojoec buffalojoec Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I've used the same implementation as the original, but I've omitted the debug! (to avoid a dependency on log) and the metrics submission. The former omission is why _account_state is unused here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the PR description, Bank's wrapper around RentCollector would then override this method to inject debug! and metrics submission.


/// Collect rent from an account.
fn collect_rent(&self, address: &Pubkey, account: &mut AccountSharedData) -> CollectedInfo;

/// Determine the rent state of an account.
///
/// This method has a default implementation that treats accounts with zero
/// lamports as uninitialized and uses the implemented `get_rent` to
/// determine whether an account is rent-exempt.
fn get_account_rent_state(&self, account: &AccountSharedData) -> RentState {
if account.lamports() == 0 {
RentState::Uninitialized
} else if self
.get_rent()
.is_exempt(account.lamports(), account.data().len())
{
RentState::RentExempt
} else {
RentState::RentPaying {
data_size: account.data().len(),
lamports: account.lamports(),
}
}
}
Comment on lines +82 to +101
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken the methods from RentState and implemented them on the trait itself to provide more customization.


/// Get the rent collector's rent instance.
fn get_rent(&self) -> &Rent;

/// Get the rent due for an account.
fn get_rent_due(&self, lamports: u64, data_len: usize, account_rent_epoch: Epoch) -> RentDue;

/// Check whether a transition from the pre_rent_state to the
/// post_rent_state is valid.
///
/// This method has a default implementation that allows transitions from
/// any state to `RentState::Uninitialized` or `RentState::RentExempt`.
/// Pre-state `RentState::RentPaying` can only transition to
/// `RentState::RentPaying` if the data size remains the same and the
/// account is not credited.
fn transition_allowed(&self, pre_rent_state: &RentState, post_rent_state: &RentState) -> bool {
match post_rent_state {
RentState::Uninitialized | RentState::RentExempt => true,
RentState::RentPaying {
data_size: post_data_size,
lamports: post_lamports,
} => {
match pre_rent_state {
RentState::Uninitialized | RentState::RentExempt => false,
RentState::RentPaying {
data_size: pre_data_size,
lamports: pre_lamports,
} => {
// Cannot remain RentPaying if resized or credited.
post_data_size == pre_data_size && post_lamports <= pre_lamports
}
}
}
}
}
}
Loading