Skip to content

Add the transaction-level account index of erroring programs to TransactionError::InstructionError #6083

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
206 changes: 13 additions & 193 deletions Cargo.lock

Large diffs are not rendered by default.

100 changes: 100 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,106 @@ zstd = "0.13.3"
opt-level = 3

[patch.crates-io]
# The following entries are auto-generated by /bin/bash
Copy link
Author

Choose a reason for hiding this comment

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

Ignore this. This will disappear when anza-xyz/solana-sdk#74 is released.

solana-account = { path = "/home/sol/src/solana-sdk/account" }
solana-account-info = { path = "/home/sol/src/solana-sdk/account-info" }
solana-address-lookup-table-interface = { path = "/home/sol/src/solana-sdk/address-lookup-table-interface" }
solana-atomic-u64 = { path = "/home/sol/src/solana-sdk/atomic-u64" }
solana-big-mod-exp = { path = "/home/sol/src/solana-sdk/big-mod-exp" }
solana-bincode = { path = "/home/sol/src/solana-sdk/bincode" }
solana-blake3-hasher = { path = "/home/sol/src/solana-sdk/blake3-hasher" }
solana-bn254 = { path = "/home/sol/src/solana-sdk/bn254" }
solana-borsh = { path = "/home/sol/src/solana-sdk/borsh" }
solana-client-traits = { path = "/home/sol/src/solana-sdk/client-traits" }
solana-clock = { path = "/home/sol/src/solana-sdk/clock" }
solana-cluster-type = { path = "/home/sol/src/solana-sdk/cluster-type" }
solana-commitment-config = { path = "/home/sol/src/solana-sdk/commitment-config" }
solana-compute-budget-interface = { path = "/home/sol/src/solana-sdk/compute-budget-interface" }
solana-cpi = { path = "/home/sol/src/solana-sdk/cpi" }
solana-decode-error = { path = "/home/sol/src/solana-sdk/decode-error" }
solana-define-syscall = { path = "/home/sol/src/solana-sdk/define-syscall" }
solana-derivation-path = { path = "/home/sol/src/solana-sdk/derivation-path" }
solana-ed25519-program = { path = "/home/sol/src/solana-sdk/ed25519-program" }
solana-epoch-info = { path = "/home/sol/src/solana-sdk/epoch-info" }
solana-epoch-rewards = { path = "/home/sol/src/solana-sdk/epoch-rewards" }
solana-epoch-rewards-hasher = { path = "/home/sol/src/solana-sdk/epoch-rewards-hasher" }
solana-epoch-schedule = { path = "/home/sol/src/solana-sdk/epoch-schedule" }
solana-example-mocks = { path = "/home/sol/src/solana-sdk/example-mocks" }
solana-feature-gate-interface = { path = "/home/sol/src/solana-sdk/feature-gate-interface" }
solana-feature-set = { path = "/home/sol/src/solana-sdk/feature-set" }
solana-fee-calculator = { path = "/home/sol/src/solana-sdk/fee-calculator" }
solana-fee-structure = { path = "/home/sol/src/solana-sdk/fee-structure" }
solana-file-download = { path = "/home/sol/src/solana-sdk/file-download" }
solana-frozen-abi = { path = "/home/sol/src/solana-sdk/frozen-abi" }
solana-frozen-abi-macro = { path = "/home/sol/src/solana-sdk/frozen-abi-macro" }
solana-genesis-config = { path = "/home/sol/src/solana-sdk/genesis-config" }
solana-hard-forks = { path = "/home/sol/src/solana-sdk/hard-forks" }
solana-hash = { path = "/home/sol/src/solana-sdk/hash" }
solana-inflation = { path = "/home/sol/src/solana-sdk/inflation" }
solana-instruction = { path = "/home/sol/src/solana-sdk/instruction" }
solana-instructions-sysvar = { path = "/home/sol/src/solana-sdk/instructions-sysvar" }
solana-keccak-hasher = { path = "/home/sol/src/solana-sdk/keccak-hasher" }
solana-keypair = { path = "/home/sol/src/solana-sdk/keypair" }
solana-last-restart-slot = { path = "/home/sol/src/solana-sdk/last-restart-slot" }
solana-loader-v2-interface = { path = "/home/sol/src/solana-sdk/loader-v2-interface" }
solana-loader-v3-interface = { path = "/home/sol/src/solana-sdk/loader-v3-interface" }
solana-loader-v4-interface = { path = "/home/sol/src/solana-sdk/loader-v4-interface" }
solana-logger = { path = "/home/sol/src/solana-sdk/logger" }
solana-message = { path = "/home/sol/src/solana-sdk/message" }
solana-msg = { path = "/home/sol/src/solana-sdk/msg" }
solana-native-token = { path = "/home/sol/src/solana-sdk/native-token" }
solana-nonce = { path = "/home/sol/src/solana-sdk/nonce" }
solana-nonce-account = { path = "/home/sol/src/solana-sdk/nonce-account" }
solana-offchain-message = { path = "/home/sol/src/solana-sdk/offchain-message" }
solana-package-metadata = { path = "/home/sol/src/solana-sdk/package-metadata" }
solana-package-metadata-macro = { path = "/home/sol/src/solana-sdk/package-metadata-macro" }
solana-packet = { path = "/home/sol/src/solana-sdk/packet" }
solana-poh-config = { path = "/home/sol/src/solana-sdk/poh-config" }
solana-precompile-error = { path = "/home/sol/src/solana-sdk/precompile-error" }
solana-precompiles = { path = "/home/sol/src/solana-sdk/precompiles" }
solana-presigner = { path = "/home/sol/src/solana-sdk/presigner" }
solana-program = { path = "/home/sol/src/solana-sdk/program" }
solana-program-entrypoint = { path = "/home/sol/src/solana-sdk/program-entrypoint" }
solana-program-error = { path = "/home/sol/src/solana-sdk/program-error" }
solana-program-memory = { path = "/home/sol/src/solana-sdk/program-memory" }
solana-program-option = { path = "/home/sol/src/solana-sdk/program-option" }
solana-program-pack = { path = "/home/sol/src/solana-sdk/program-pack" }
solana-pubkey = { path = "/home/sol/src/solana-sdk/pubkey" }
solana-quic-definitions = { path = "/home/sol/src/solana-sdk/quic-definitions" }
solana-rent = { path = "/home/sol/src/solana-sdk/rent" }
solana-rent-collector = { path = "/home/sol/src/solana-sdk/rent-collector" }
solana-rent-debits = { path = "/home/sol/src/solana-sdk/rent-debits" }
solana-reserved-account-keys = { path = "/home/sol/src/solana-sdk/reserved-account-keys" }
solana-reward-info = { path = "/home/sol/src/solana-sdk/reward-info" }
solana-sanitize = { path = "/home/sol/src/solana-sdk/sanitize" }
solana-sdk = { path = "/home/sol/src/solana-sdk/sdk" }
solana-sdk-ids = { path = "/home/sol/src/solana-sdk/sdk-ids" }
solana-sdk-macro = { path = "/home/sol/src/solana-sdk/sdk-macro" }
solana-secp256k1-program = { path = "/home/sol/src/solana-sdk/secp256k1-program" }
solana-secp256k1-recover = { path = "/home/sol/src/solana-sdk/secp256k1-recover" }
solana-secp256r1-program = { path = "/home/sol/src/solana-sdk/secp256r1-program" }
solana-seed-derivable = { path = "/home/sol/src/solana-sdk/seed-derivable" }
solana-seed-phrase = { path = "/home/sol/src/solana-sdk/seed-phrase" }
solana-serde = { path = "/home/sol/src/solana-sdk/serde" }
solana-serde-varint = { path = "/home/sol/src/solana-sdk/serde-varint" }
solana-serialize-utils = { path = "/home/sol/src/solana-sdk/serialize-utils" }
solana-sha256-hasher = { path = "/home/sol/src/solana-sdk/sha256-hasher" }
solana-short-vec = { path = "/home/sol/src/solana-sdk/short-vec" }
solana-shred-version = { path = "/home/sol/src/solana-sdk/shred-version" }
solana-signature = { path = "/home/sol/src/solana-sdk/signature" }
solana-signer = { path = "/home/sol/src/solana-sdk/signer" }
solana-slot-hashes = { path = "/home/sol/src/solana-sdk/slot-hashes" }
solana-slot-history = { path = "/home/sol/src/solana-sdk/slot-history" }
solana-stable-layout = { path = "/home/sol/src/solana-sdk/stable-layout" }
solana-system-transaction = { path = "/home/sol/src/solana-sdk/system-transaction" }
solana-sysvar = { path = "/home/sol/src/solana-sdk/sysvar" }
solana-sysvar-id = { path = "/home/sol/src/solana-sdk/sysvar-id" }
solana-time-utils = { path = "/home/sol/src/solana-sdk/time-utils" }
solana-transaction = { path = "/home/sol/src/solana-sdk/transaction" }
solana-transaction-error = { path = "/home/sol/src/solana-sdk/transaction-error" }
solana-validator-exit = { path = "/home/sol/src/solana-sdk/validator-exit" }
solana-vote-interface = { path = "/home/sol/src/solana-sdk/vote-interface" }

# for details, see https://github.com/anza-xyz/crossbeam/commit/fd279d707025f0e60951e429bf778b4813d1b6bf
crossbeam-epoch = { git = "https://github.com/anza-xyz/crossbeam", rev = "fd279d707025f0e60951e429bf778b4813d1b6bf" }

Expand Down
2 changes: 1 addition & 1 deletion cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,7 @@ where
match result {
Err(err) => {
let maybe_tx_err = err.get_transaction_error();
if let Some(TransactionError::InstructionError(_, ix_error)) = maybe_tx_err {
if let Some(TransactionError::InstructionError(_, ix_error, _, _)) = maybe_tx_err {
if let Some(specific_error) = error_adapter(&ix_error) {
return Err(specific_error.into());
}
Expand Down
8 changes: 8 additions & 0 deletions cli/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2214,12 +2214,16 @@ fn close(
if let ClientErrorKind::TransactionError(TransactionError::InstructionError(
_,
InstructionError::InvalidInstructionData,
_,
_,
)) = err.kind()
{
return Err("Closing a buffer account is not supported by the cluster".into());
} else if let ClientErrorKind::TransactionError(TransactionError::InstructionError(
_,
InstructionError::InvalidArgument,
_,
_,
)) = err.kind()
{
return Err("Closing a program account is not supported by the cluster".into());
Expand Down Expand Up @@ -2437,6 +2441,8 @@ fn process_extend_program(
if let ClientErrorKind::TransactionError(TransactionError::InstructionError(
_,
InstructionError::InvalidInstructionData,
_,
_,
)) = err.kind()
{
return Err("Extending a program is not supported by the cluster".into());
Expand Down Expand Up @@ -2532,6 +2538,8 @@ fn process_migrate_program(
if let ClientErrorKind::TransactionError(TransactionError::InstructionError(
_,
InstructionError::InvalidInstructionData,
_,
_,
)) = err.kind()
{
return Err("Migrating a program is not supported by the cluster".into());
Expand Down
1 change: 1 addition & 0 deletions compute-budget-instruction/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ crate-type = ["lib"]
name = "solana_compute_budget_instruction"

[dev-dependencies]
assert_matches = { workspace = true }

Choose a reason for hiding this comment

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

Was this change intended?

bincode = { workspace = true }
criterion = { workspace = true }
rand = { workspace = true }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ impl ComputeBudgetInstructionDetails {
return Err(TransactionError::InstructionError(
index,
InstructionError::InvalidInstructionData,
None,
None,
));
}
} else {
Expand Down Expand Up @@ -153,8 +155,12 @@ impl ComputeBudgetInstructionDetails {
}

fn process_instruction(&mut self, index: u8, instruction: &SVMInstruction) -> Result<()> {
let invalid_instruction_data_error =
TransactionError::InstructionError(index, InstructionError::InvalidInstructionData);
let invalid_instruction_data_error = TransactionError::InstructionError(
index,
InstructionError::InvalidInstructionData,
None,
Some(instruction.program_id_index),
Copy link
Author

Choose a reason for hiding this comment

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

It seems that the SVMInstruction already knows the transaction-wide index of its program, but please double check me on this. cc/ @buffalojoec

https://github.com/steveluscher/agave/blob/f7a6f80ff632216ceb82bcbd95a9a8c1d9855987/svm-transaction/src/instruction.rs#L8-L9

);
let duplicate_instruction_error = TransactionError::DuplicateInstruction(index);

match try_from_slice_unchecked(instruction.data) {
Expand Down Expand Up @@ -407,6 +413,8 @@ mod test {
let expected_heap_size_err = Err(TransactionError::InstructionError(
3,
InstructionError::InvalidInstructionData,
None,
None,
));
// invalid: requested_heap_size can't be zero
let instruction_details = ComputeBudgetInstructionDetails {
Expand Down
8 changes: 8 additions & 0 deletions compute-budget-instruction/src/instructions_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ mod tests {
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidInstructionData,
None,
None,
))
);
test!(
Expand All @@ -174,6 +176,8 @@ mod tests {
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidInstructionData,
None,
None,
))
);
test!(
Expand All @@ -184,6 +188,8 @@ mod tests {
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidInstructionData,
None,
None,
))
);
test!(
Expand Down Expand Up @@ -222,6 +228,8 @@ mod tests {
Err(TransactionError::InstructionError(
3,
InstructionError::InvalidInstructionData,
None,
None,
))
);
test!(
Expand Down
15 changes: 9 additions & 6 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1573,17 +1573,20 @@ mod tests {
(transaction.signatures[0], meta.status)
})
.collect();
let expected_tx_results = vec![
(success_signature, Ok(())),
assert_eq!(actual_tx_results.len(), 2);
assert_eq!(actual_tx_results[0], (success_signature, Ok(())));
assert_matches!(
actual_tx_results[1],
(
ix_error_signature,
actual_ix_signature,
Err(TransactionError::InstructionError(
0,
InstructionError::Custom(1),
None,
Some(ii),
)),
),
];
assert_eq!(actual_tx_results, expected_tx_results);
) if ii > 0 && actual_ix_signature == ix_error_signature
);

poh_recorder
.read()
Expand Down
17 changes: 11 additions & 6 deletions core/src/replay_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5347,17 +5347,22 @@ pub(crate) mod tests {
(transaction.signatures[0], meta.status)
})
.collect();
let expected_tx_results = vec![
(test_signatures_iter.next().unwrap(), Ok(())),
assert_eq!(actual_tx_results.len(), 2);
let first_expected_result = test_signatures_iter.next().unwrap();
assert_eq!(actual_tx_results[0], (first_expected_result, Ok(())));
let second_expected_result = test_signatures_iter.next().unwrap();
assert_matches!(
actual_tx_results[1],
(
test_signatures_iter.next().unwrap(),
actual_result,
Err(TransactionError::InstructionError(
0,
InstructionError::Custom(1),
None,
Some(ii),
)),
),
];
assert_eq!(actual_tx_results, expected_tx_results);
) if ii > 0 && actual_result == second_expected_result
);
assert!(test_signatures_iter.next().is_none());
}
Blockstore::destroy(&ledger_path).unwrap();
Expand Down
43 changes: 23 additions & 20 deletions core/tests/scheduler_cost_adjustment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![cfg(test)]
use {
assert_matches::assert_matches,
solana_compute_budget::compute_budget_limits::MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT,
solana_cost_model::cost_model::CostModel,
solana_runtime::{bank::Bank, bank_forks::BankForks},
Expand Down Expand Up @@ -230,19 +231,20 @@ fn test_builtin_ix_cost_adjustment_with_cu_limit_too_low() {
// Cost model & Compute budget: reserve/allocate requested CU Limit `1`
// VM Execution: consume `1` CU, then fail
// Result: 0 adjustment
let expected = TestResult {
cost_adjustment: 0,
execution_status: Err(TransactionError::InstructionError(
0,
InstructionError::ComputationalBudgetExceeded,
)),
};
assert_eq!(
expected,
assert_matches!(
test_setup.execute_test_transaction(&[
test_setup.transfer_ix(),
test_setup.set_cu_limit_ix(cu_limit),
])
]),
TestResult {
cost_adjustment: 0,
execution_status: Err(TransactionError::InstructionError(
0,
InstructionError::ComputationalBudgetExceeded,
None,
Some(ii),
)),
} if ii > 0
);
}

Expand Down Expand Up @@ -282,16 +284,17 @@ fn test_builtin_ix_cost_adjustment_with_memo_no_cu_limit() {
// (3_000 + 200_000) = 203_000 CUs (note: less than memo_ix needs)
// VM Execution: consume all allocated CUs, then fail
// Result: no adjustment
let expected = TestResult {
cost_adjustment: 0,
execution_status: Err(TransactionError::InstructionError(
1,
InstructionError::ProgramFailedToComplete,
)),
};
assert_eq!(
expected,
test_setup.execute_test_transaction(&[test_setup.transfer_ix(), memo_ix.clone()],)
assert_matches!(
test_setup.execute_test_transaction(&[test_setup.transfer_ix(), memo_ix.clone()],),
TestResult {
cost_adjustment: 0,
execution_status: Err(TransactionError::InstructionError(
1,
InstructionError::ProgramFailedToComplete,
None,
Some(ii),
)),
} if ii > 0
);
}

Expand Down
8 changes: 5 additions & 3 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3995,12 +3995,14 @@ pub mod tests {
bank.transfer(1_000, &mint_keypair, &pubkey).unwrap();
assert_eq!(bank.transaction_count(), 1);
assert_eq!(bank.get_balance(&pubkey), 1_000);
assert_eq!(
assert_matches!(
bank.transfer(10_001, &mint_keypair, &pubkey),
Err(TransactionError::InstructionError(
0,
SystemError::ResultWithNegativeLamports.into(),
))
actual_err,
None,
Some(ii),
)) if ii > 0 && actual_err == SystemError::ResultWithNegativeLamports.into()
);
assert_eq!(
bank.transfer(10_001, &mint_keypair, &pubkey),
Expand Down
10 changes: 8 additions & 2 deletions program-test/tests/panic.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use {
assert_matches::assert_matches,
solana_program_test::{processor, ProgramTest},
solana_sdk::{
account_info::AccountInfo,
Expand Down Expand Up @@ -30,13 +31,18 @@ async fn panic_test() {
&[&context.payer],
context.last_blockhash,
);
assert_eq!(
assert_matches!(
context
.banks_client
.process_transaction(transaction)
.await
.unwrap_err()
.unwrap(),
TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete)
TransactionError::InstructionError(
0,
InstructionError::ProgramFailedToComplete,
None,
Some(ii),
) if ii > 0
);
}
Loading