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

Refactor - Avoid host build of SBPF program test crates #1711

Merged
1 change: 1 addition & 0 deletions ci/test-stable.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ test-stable-sbf)
fi

# SBF C program system tests
export SBF_OUT_DIR=target/sbf-solana-solana/release
_ make -C programs/sbf/c tests
_ cargo test \
--manifest-path programs/sbf/Cargo.toml \
Expand Down
6 changes: 0 additions & 6 deletions programs/sbf/Cargo.lock

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

12 changes: 0 additions & 12 deletions programs/sbf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,15 @@ solana-measure = { path = "../../measure", version = "=2.0.0" }
solana-poseidon = { path = "../../poseidon/", version = "=2.0.0" }
solana-program = { path = "../../sdk/program", version = "=2.0.0" }
solana-program-runtime = { path = "../../program-runtime", version = "=2.0.0" }
solana-program-test = { path = "../../program-test", version = "=2.0.0" }
solana-runtime = { path = "../../runtime", version = "=2.0.0" }
solana-sbf-rust-128bit-dep = { path = "rust/128bit_dep", version = "=2.0.0" }
solana-sbf-rust-invoke-dep = { path = "rust/invoke_dep", version = "=2.0.0" }
solana-sbf-rust-invoked-dep = { path = "rust/invoked_dep", version = "=2.0.0" }
solana-sbf-rust-many-args-dep = { path = "rust/many_args_dep", version = "=2.0.0" }
solana-sbf-rust-mem = { path = "rust/mem", version = "=2.0.0" }
solana-sbf-rust-mem-dep = { path = "rust/mem_dep", version = "=2.0.0" }
solana-sbf-rust-param-passing-dep = { path = "rust/param_passing_dep", version = "=2.0.0" }
solana-sbf-rust-realloc-dep = { path = "rust/realloc_dep", version = "=2.0.0" }
solana-sbf-rust-realloc-invoke-dep = { path = "rust/realloc_invoke_dep", version = "=2.0.0" }
solana-sbf-rust-remaining-compute-units = { path = "rust/remaining_compute_units", version = "=2.0.0" }
solana-sbf-rust-sanity = { path = "rust/sanity", version = "=2.0.0" }
solana-sbf-rust-simulation = { path = "rust/simulation", version = "=2.0.0" }
solana-sbf-rust-sysvar = { path = "rust/sysvar", version = "=2.0.0" }
solana-sdk = { path = "../../sdk", version = "=2.0.0" }
solana-svm = { path = "../../svm", version = "=2.0.0" }
solana-transaction-status = { path = "../../transaction-status", version = "=2.0.0" }
Expand Down Expand Up @@ -100,16 +94,10 @@ solana-logger = { workspace = true }
solana-measure = { workspace = true }
solana-program = { workspace = true }
solana-program-runtime = { workspace = true }
solana-program-test = { workspace = true }
solana-runtime = { workspace = true, features = ["dev-context-only-utils"] }
solana-sbf-rust-invoke-dep = { workspace = true }
solana-sbf-rust-mem = { workspace = true }
solana-sbf-rust-realloc-dep = { workspace = true }
solana-sbf-rust-realloc-invoke-dep = { workspace = true }
solana-sbf-rust-remaining-compute-units = { workspace = true }
solana-sbf-rust-sanity = { workspace = true }
solana-sbf-rust-simulation = { workspace = true }
solana-sbf-rust-sysvar = { workspace = true }
solana-sdk = { workspace = true, features = ["dev-context-only-utils"] }
solana-svm = { workspace = true }
solana-transaction-status = { workspace = true }
Expand Down
1 change: 1 addition & 0 deletions programs/sbf/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ fn main() {
"simulation",
"spoof1",
"spoof1_system",
"sysvar",
"upgradeable",
"upgraded",
];
Expand Down
2 changes: 1 addition & 1 deletion programs/sbf/rust/mem/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ solana-program = { workspace = true }
solana-sbf-rust-mem-dep = { workspace = true }

[lib]
crate-type = ["cdylib", "lib"]
crate-type = ["cdylib"]
2 changes: 1 addition & 1 deletion programs/sbf/rust/remaining_compute_units/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ edition = { workspace = true }
solana-program = { workspace = true }

[lib]
crate-type = ["cdylib", "lib"]
crate-type = ["cdylib"]
2 changes: 1 addition & 1 deletion programs/sbf/rust/sanity/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ edition = { workspace = true }
solana-program = { workspace = true }

[lib]
crate-type = ["cdylib", "lib"]
crate-type = ["cdylib"]
2 changes: 1 addition & 1 deletion programs/sbf/rust/simulation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ edition = { workspace = true }
solana-program = { workspace = true }

[lib]
crate-type = ["cdylib", "lib"]
crate-type = ["cdylib"]
2 changes: 1 addition & 1 deletion programs/sbf/rust/sysvar/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ edition = { workspace = true }
solana-program = { workspace = true }

[lib]
crate-type = ["cdylib", "lib"]
crate-type = ["cdylib"]
16 changes: 8 additions & 8 deletions programs/sbf/rust/sysvar/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub fn process_instruction(
sysvar::clock::id().log();
let clock = Clock::from_account_info(&accounts[2]).unwrap();
assert_ne!(clock, Clock::default());
let got_clock = Clock::get()?;
let got_clock = Clock::get().unwrap();

Choose a reason for hiding this comment

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

Maybe I'm missing something, but what are these panics supposed to catch that the error doesn't? The test is just making sure the result is Ok(()), so unless you're debugging I would assume this is no different than the test failing on the thrown error.

Copy link
Author

Choose a reason for hiding this comment

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

Yes looks funny I know. All of these ? => .unwrap() changes are in the commit Workaround for rustc crash caused by 16 byte alined memcpy.

Lucas is already working on fixing that, but it takes one release cycle of the platform tools, so this is the workaround for now.

Choose a reason for hiding this comment

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

How does this workaround actually work, though?

Copy link
Author

Choose a reason for hiding this comment

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

The memcpy is not explicit it happens because of stack spilling / register pressure as far as I can tell. So pretty much any change to the code can cause it to go either way. With the next release of the platform tools that should be fixed, and until then the behavior is stable.

assert_eq!(clock, got_clock);
}

Expand All @@ -41,16 +41,17 @@ pub fn process_instruction(
sysvar::epoch_schedule::id().log();
let epoch_schedule = EpochSchedule::from_account_info(&accounts[3]).unwrap();
assert_eq!(epoch_schedule, EpochSchedule::default());
let got_epoch_schedule = EpochSchedule::get()?;
let got_epoch_schedule = EpochSchedule::get().unwrap();
assert_eq!(epoch_schedule, got_epoch_schedule);
}

// Instructions
msg!("Instructions identifier:");
sysvar::instructions::id().log();
assert_eq!(*accounts[4].owner, sysvar::id());
let index = instructions::load_current_index_checked(&accounts[4])?;
let instruction = instructions::load_instruction_at_checked(index as usize, &accounts[4])?;
let index = instructions::load_current_index_checked(&accounts[4]).unwrap();
let instruction =
instructions::load_instruction_at_checked(index as usize, &accounts[4]).unwrap();
assert_eq!(0, index);
assert_eq!(
instruction,
Expand Down Expand Up @@ -88,8 +89,7 @@ pub fn process_instruction(
msg!("Rent identifier:");
sysvar::rent::id().log();
let rent = Rent::from_account_info(&accounts[6]).unwrap();
assert_eq!(rent, Rent::default());
buffalojoec marked this conversation as resolved.
Show resolved Hide resolved
let got_rent = Rent::get()?;
let got_rent = Rent::get().unwrap();
assert_eq!(rent, got_rent);
}

Expand Down Expand Up @@ -120,7 +120,7 @@ pub fn process_instruction(
msg!("Fee identifier:");
sysvar::fees::id().log();
let fees = Fees::from_account_info(&accounts[10]).unwrap();
let got_fees = Fees::get()?;
let got_fees = Fees::get().unwrap();
assert_eq!(fees, got_fees);
}

Expand All @@ -129,7 +129,7 @@ pub fn process_instruction(
msg!("EpochRewards identifier:");
sysvar::epoch_rewards::id().log();
let epoch_rewards = EpochRewards::from_account_info(&accounts[11]).unwrap();
let got_epoch_rewards = EpochRewards::get()?;
let got_epoch_rewards = EpochRewards::get().unwrap();
assert_eq!(epoch_rewards, got_epoch_rewards);
}

Expand Down
27 changes: 0 additions & 27 deletions programs/sbf/tests/mem.rs

This file was deleted.

2 changes: 2 additions & 0 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,14 @@ fn test_program_sbf_sanity() {
("solana_sbf_rust_external_spend", false),
("solana_sbf_rust_iter", true),
("solana_sbf_rust_many_args", true),
("solana_sbf_rust_mem", true),
("solana_sbf_rust_membuiltins", true),
("solana_sbf_rust_noop", true),
("solana_sbf_rust_panic", false),
("solana_sbf_rust_param_passing", true),
("solana_sbf_rust_poseidon", true),
("solana_sbf_rust_rand", true),
("solana_sbf_rust_remaining_compute_units", true),
("solana_sbf_rust_sanity", true),
("solana_sbf_rust_secp256k1_recover", true),
("solana_sbf_rust_sha", true),
Expand Down
27 changes: 0 additions & 27 deletions programs/sbf/tests/remaining_compute_units.rs

This file was deleted.

37 changes: 0 additions & 37 deletions programs/sbf/tests/sanity.rs

This file was deleted.

86 changes: 64 additions & 22 deletions programs/sbf/tests/simulation.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,86 @@
#![cfg(feature = "test-bpf")]

use {
solana_program_test::{processor, tokio, ProgramTest},
solana_sbf_rust_simulation::process_instruction,
agave_validator::test_validator::*,
solana_runtime::{
bank::Bank,
bank_client::BankClient,
genesis_utils::{create_genesis_config, GenesisConfigInfo},
loader_utils::load_upgradeable_program_and_advance_slot,
},
solana_sdk::{
instruction::{AccountMeta, Instruction},
message::Message,
pubkey::Pubkey,
signature::Signer,
sysvar,
transaction::Transaction,
signature::{Keypair, Signer},
sysvar::{clock, slot_history},
transaction::{SanitizedTransaction, Transaction},
},
};

#[tokio::test]
async fn no_panic_banks_client() {
let program_id = Pubkey::new_unique();
let program_test = ProgramTest::new(
#[test]
#[cfg(feature = "sbf_rust")]
fn test_no_panic_banks_client() {
solana_logger::setup();

let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = create_genesis_config(50);
let (bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);
let mut bank_client = BankClient::new_shared(bank.clone());
let authority_keypair = Keypair::new();
let (bank, program_id) = load_upgradeable_program_and_advance_slot(
&mut bank_client,
bank_forks.as_ref(),
&mint_keypair,
&authority_keypair,
"solana_sbf_rust_simulation",
);
bank.freeze();

let instruction = Instruction::new_with_bincode(
program_id,
processor!(process_instruction),
&[0u8; 0],
vec![
AccountMeta::new_readonly(slot_history::id(), false),
AccountMeta::new_readonly(clock::id(), false),
],
);
let blockhash = bank.last_blockhash();
let message = Message::new(&[instruction], Some(&mint_keypair.pubkey()));
let transaction = Transaction::new(&[&mint_keypair], message, blockhash);
let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(transaction);
let result = bank.simulate_transaction(&sanitized_tx, false);
assert!(result.result.is_ok());
}

#[test]
#[cfg(feature = "sbf_rust")]
fn test_no_panic_rpc_client() {
solana_logger::setup();

let program_id = Pubkey::new_unique();
let (test_validator, payer) = TestValidatorGenesis::default()
.add_program("solana_sbf_rust_simulation", program_id)
.start();
let rpc_client = test_validator.get_rpc_client();
let blockhash = rpc_client.get_latest_blockhash().unwrap();

let mut context = program_test.start_with_context().await;
let transaction = Transaction::new_signed_with_payer(
&[Instruction {
program_id,
accounts: vec![
AccountMeta::new_readonly(sysvar::slot_history::id(), false),
AccountMeta::new_readonly(sysvar::clock::id(), false),
AccountMeta::new_readonly(slot_history::id(), false),
AccountMeta::new_readonly(clock::id(), false),
],
data: vec![],
}],
Some(&context.payer.pubkey()),
&[&context.payer],
context.last_blockhash,
Some(&payer.pubkey()),
&[&payer],
blockhash,
);

context
.banks_client
.process_transaction_with_preflight(transaction)
.await
rpc_client
.send_and_confirm_transaction(&transaction)
buffalojoec marked this conversation as resolved.
Show resolved Hide resolved
.unwrap();
}
Loading