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

Extract account-info crate #2429

Merged
merged 8 commits into from
Sep 25, 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
12 changes: 12 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 @@ -101,6 +101,7 @@ members = [
"runtime-transaction",
"sanitize",
"sdk",
"sdk/account-info",
"sdk/atomic-u64",
"sdk/cargo-build-sbf",
"sdk/cargo-test-sbf",
Expand Down Expand Up @@ -357,6 +358,7 @@ smpl_jwt = "0.7.1"
socket2 = "0.5.7"
soketto = "0.7"
solana-account-decoder = { path = "account-decoder", version = "=2.1.0" }
solana-account-info = { path = "sdk/account-info", version = "=2.1.0" }
solana-accounts-db = { path = "accounts-db", version = "=2.1.0" }
solana-address-lookup-table-program = { path = "programs/address-lookup-table", version = "=2.1.0" }
solana-atomic-u64 = { path = "sdk/atomic-u64", version = "=2.1.0" }
Expand Down
12 changes: 12 additions & 0 deletions programs/sbf/Cargo.lock

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

23 changes: 23 additions & 0 deletions sdk/account-info/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[package]
name = "solana-account-info"
description = "Solana AccountInfo and related definitions."
documentation = "https://docs.rs/solana-account-info"
version = { workspace = true }
authors = { workspace = true }
repository = { workspace = true }
homepage = { workspace = true }
license = { workspace = true }
edition = { workspace = true }

[dependencies]
bincode = { workspace = true, optional = true }
serde = { workspace = true, optional = true }
solana-program-error = { workspace = true }
solana-program-memory = { workspace = true }
solana-pubkey = { workspace = true, default-features = false }

[features]
bincode = ["dep:bincode", "dep:serde"]

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
46 changes: 24 additions & 22 deletions sdk/program/src/account_info.rs → sdk/account-info/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
//! Account information.

use {
crate::{
debug_account_data::*, entrypoint::MAX_PERMITTED_DATA_INCREASE,
program_error::ProgramError, pubkey::Pubkey,
},
solana_clock::Epoch,
solana_program_error::ProgramError,
solana_program_memory::sol_memset,
solana_pubkey::Pubkey,
std::{
cell::{Ref, RefCell, RefMut},
fmt,
rc::Rc,
slice::from_raw_parts_mut,
},
};
pub mod debug_account_data;

/// Maximum number of bytes a program may add to an account during a single realloc
pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10;

/// Account information
#[derive(Clone)]
Expand All @@ -28,7 +29,7 @@ pub struct AccountInfo<'a> {
/// Program that owns this account
pub owner: &'a Pubkey,
/// The epoch at which this account will next owe rent
pub rent_epoch: Epoch,
pub rent_epoch: u64,

Choose a reason for hiding this comment

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

Why was the Epoch type replaced with u64? Since we're using Pubkey, can we keep using Epoch too, please?

Choose a reason for hiding this comment

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

Pubkey is a struct, and not a type, so it would be a breaking change to use anything else.

I might be missing something, but what's the reasoning to prefer the Epoch type alias? The idea was to avoid an extra dependency.

Choose a reason for hiding this comment

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

Pubkey is a struct, and not a type, so it would be a breaking change to use anything else.

A struct is a type ;)

I'd prefer if Epoch were a proper type too, and not just an alias. (But that's orthogonal to this discussion.)


The idea was to avoid an extra dependency.

Ah, that part makes sense. How "expensive" is it to have this extra dependency?


what's the reasoning to prefer the Epoch type alias?

Strong typing is beneficial for preventing errors/making refactors easier/etc. Since Epoch is a type alias, we lose some of that, but we still retain search and the self-documenting aspects of Epoch.

Choose a reason for hiding this comment

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

Ah, that part makes sense. How "expensive" is it to have this extra dependency?

Maybe @kevinheavey has some numbers on that, or I can whip them up

Since Epoch is a type alias, we lose some of that, but we still retain search and the self-documenting aspects of Epoch.

Gotcha, I can see from the documenting aspect that it could be useful. My thinking was that:

  • I've never seen the rent epoch used by a program
  • it introduces another dependency
  • since it's just a type alias, it doesn't really matter

For those reasons, I lean towards keeping it as is, but if you feel strongly about it, we can change it.

Choose a reason for hiding this comment

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

These points make sense to me, especially since this type is meant to be program-facing. The internal-facing structs for accounts can/do keep using Epoch, so I don't think any changes are necessary. Thanks for taking the time to explain this to me!

Choose a reason for hiding this comment

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

Sounds good! And thanks for clarifying the difference between type and type alias 😅

Copy link
Author

Choose a reason for hiding this comment

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

yeah on my machine adding the solana-clock dependency increases solana-account-info build time by 40%. It's made quite expensive by the proc macro deps in solana-clock

/// Was the transaction signed by this account's public key?
pub is_signer: bool,
/// Is the account writable?
Expand All @@ -49,7 +50,7 @@ impl<'a> fmt::Debug for AccountInfo<'a> {
.field("rent_epoch", &self.rent_epoch)
.field("lamports", &self.lamports())
.field("data.len", &self.data_len());
debug_account_data(&self.data.borrow(), &mut f);
debug_account_data::debug_account_data(&self.data.borrow(), &mut f);

f.finish_non_exhaustive()
}
Expand Down Expand Up @@ -203,7 +204,7 @@ impl<'a> AccountInfo<'a> {
data: &'a mut [u8],
owner: &'a Pubkey,
executable: bool,
rent_epoch: Epoch,
rent_epoch: u64,
) -> Self {
Self {
key,
Expand All @@ -217,10 +218,12 @@ impl<'a> AccountInfo<'a> {
}
}

#[cfg(feature = "bincode")]
pub fn deserialize_data<T: serde::de::DeserializeOwned>(&self) -> Result<T, bincode::Error> {
bincode::deserialize(&self.data.borrow())
}

#[cfg(feature = "bincode")]
pub fn serialize_data<T: serde::Serialize>(&self, state: &T) -> Result<(), bincode::Error> {
if bincode::serialized_size(state)? > self.data_len() as u64 {
return Err(Box::new(bincode::ErrorKind::SizeLimit));
Expand All @@ -242,7 +245,7 @@ impl<'a, T: IntoAccountInfo<'a>> From<T> for AccountInfo<'a> {
/// Provides information required to construct an `AccountInfo`, used in
/// conversion implementations.
pub trait Account {
fn get(&mut self) -> (&mut u64, &mut [u8], &Pubkey, bool, Epoch);
fn get(&mut self) -> (&mut u64, &mut [u8], &Pubkey, bool, u64);
}

/// Convert (&'a Pubkey, &'a mut T) where T: Account into an `AccountInfo`
Expand Down Expand Up @@ -293,12 +296,10 @@ impl<'a, T: Account> IntoAccountInfo<'a> for &'a mut (Pubkey, T) {
/// # Examples
///
/// ```
/// use solana_program::{
/// account_info::{AccountInfo, next_account_info},
/// entrypoint::ProgramResult,
/// pubkey::Pubkey,
/// };
/// # use solana_program::program_error::ProgramError;
/// use solana_program_error::ProgramResult;
/// use solana_account_info::{AccountInfo, next_account_info};
/// use solana_pubkey::Pubkey;
/// # use solana_program_error::ProgramError;
///
/// pub fn process_instruction(
/// program_id: &Pubkey,
Expand Down Expand Up @@ -344,12 +345,10 @@ pub fn next_account_info<'a, 'b, I: Iterator<Item = &'a AccountInfo<'b>>>(
/// # Examples
///
/// ```
/// use solana_program::{
/// account_info::{AccountInfo, next_account_info, next_account_infos},
/// entrypoint::ProgramResult,
/// pubkey::Pubkey,
/// };
/// # use solana_program::program_error::ProgramError;
/// use solana_program_error::ProgramResult;
/// use solana_account_info::{AccountInfo, next_account_info, next_account_infos};
/// use solana_pubkey::Pubkey;
/// # use solana_program_error::ProgramError;
///
/// pub fn process_instruction(
/// program_id: &Pubkey,
Expand Down Expand Up @@ -398,7 +397,10 @@ impl<'a> AsRef<AccountInfo<'a>> for AccountInfo<'a> {

#[cfg(test)]
mod tests {
use super::*;
use {
super::*,
crate::debug_account_data::{Hex, MAX_DEBUG_ACCOUNT_DATA},
};

#[test]
fn test_next_account_infos() {
Expand Down
1 change: 1 addition & 0 deletions sdk/program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ serde_bytes = { workspace = true }
serde_derive = { workspace = true }
sha2 = { workspace = true }
sha3 = { workspace = true }
solana-account-info = { workspace = true, features = ["bincode"] }
solana-atomic-u64 = { workspace = true }
solana-clock = { workspace = true, features = ["serde"] }
solana-decode-error = { workspace = true }
Expand Down
5 changes: 1 addition & 4 deletions sdk/program/src/entrypoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
//! [`bpf_loader`]: crate::bpf_loader

extern crate alloc;
pub use solana_program_error::ProgramResult;
use {
crate::{account_info::AccountInfo, pubkey::Pubkey},
alloc::vec::Vec,
Expand All @@ -18,6 +17,7 @@ use {
slice::{from_raw_parts, from_raw_parts_mut},
},
};
pub use {solana_account_info::MAX_PERMITTED_DATA_INCREASE, solana_program_error::ProgramResult};

/// User implemented function to process an instruction
///
Expand Down Expand Up @@ -310,9 +310,6 @@ unsafe impl std::alloc::GlobalAlloc for BumpAllocator {
}
}

/// Maximum number of bytes a program may add to an account during a single realloc
pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10;

/// `assert_eq(std::mem::align_of::<u128>(), 8)` is true for BPF but not for some host machines
pub const BPF_ALIGN_OF_U128: usize = 8;

Expand Down
8 changes: 4 additions & 4 deletions sdk/program/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,6 @@
// Allows macro expansion of `use ::solana_program::*` to work within this crate
extern crate self as solana_program;

pub mod account_info;
pub mod address_lookup_table;
pub mod big_mod_exp;
pub mod blake3;
Expand All @@ -483,7 +482,6 @@ pub mod bpf_loader;
pub mod bpf_loader_deprecated;
pub mod bpf_loader_upgradeable;
pub mod compute_units;
pub mod debug_account_data;
pub mod ed25519_program;
pub mod entrypoint;
pub mod entrypoint_deprecated;
Expand Down Expand Up @@ -539,8 +537,10 @@ pub use solana_short_vec as short_vec;
#[cfg(target_arch = "wasm32")]
pub use wasm_bindgen::prelude::wasm_bindgen;
pub use {
solana_clock as clock, solana_msg::msg, solana_program_option as program_option,
solana_pubkey as pubkey,
solana_account_info::{self as account_info, debug_account_data},
solana_clock as clock,
solana_msg::msg,
solana_program_option as program_option, solana_pubkey as pubkey,
};

/// The [config native program][np].
Expand Down
Loading