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

Add RPC support for versioned transactions #22530

Merged
merged 4 commits into from
Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions cli/src/cluster_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2019,6 +2019,7 @@ pub fn process_transaction_history(
RpcTransactionConfig {
encoding: Some(UiTransactionEncoding::Base64),
commitment: Some(CommitmentConfig::confirmed()),
max_supported_transaction_version: None,
},
) {
Ok(confirmed_transaction) => {
Expand Down
1 change: 1 addition & 0 deletions cli/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ pub fn process_confirm(
RpcTransactionConfig {
encoding: Some(UiTransactionEncoding::Base64),
commitment: Some(CommitmentConfig::confirmed()),
max_supported_transaction_version: None,
},
) {
Ok(confirmed_transaction) => {
Expand Down
33 changes: 21 additions & 12 deletions client-test/tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use {
solana_ledger::{blockstore::Blockstore, get_tmp_ledger_path},
solana_rpc::{
optimistically_confirmed_bank_tracker::OptimisticallyConfirmedBank,
rpc::create_test_transactions_and_populate_blockstore,
rpc::{create_test_transaction_entries, populate_blockstore_for_tests},
rpc_pubsub_service::{PubSubConfig, PubSubService},
rpc_subscriptions::RpcSubscriptions,
},
Expand All @@ -36,7 +36,9 @@ use {
},
solana_streamer::socket::SocketAddrSpace,
solana_test_validator::TestValidator,
solana_transaction_status::{ConfirmedBlock, TransactionDetails, UiTransactionEncoding},
solana_transaction_status::{
BlockEncodingOptions, ConfirmedBlock, TransactionDetails, UiTransactionEncoding,
},
std::{
collections::HashSet,
net::{IpAddr, SocketAddr},
Expand Down Expand Up @@ -230,9 +232,12 @@ fn test_block_subscription() {
let max_complete_transaction_status_slot = Arc::new(AtomicU64::new(blockstore.max_root()));
bank.transfer(rent_exempt_amount, &alice, &keypair2.pubkey())
.unwrap();
let _confirmed_block_signatures = create_test_transactions_and_populate_blockstore(
vec![&alice, &keypair1, &keypair2, &keypair3],
0,
populate_blockstore_for_tests(
create_test_transaction_entries(
vec![&alice, &keypair1, &keypair2, &keypair3],
bank.clone(),
)
.0,
bank,
blockstore.clone(),
max_complete_transaction_status_slot,
Expand Down Expand Up @@ -270,6 +275,7 @@ fn test_block_subscription() {
encoding: Some(UiTransactionEncoding::Json),
transaction_details: Some(TransactionDetails::Signatures),
show_rewards: None,
max_supported_transaction_version: None,
}),
)
.unwrap();
Expand All @@ -281,14 +287,17 @@ fn test_block_subscription() {
match maybe_actual {
Ok(actual) => {
let versioned_block = blockstore.get_complete_block(slot, false).unwrap();
let legacy_block = ConfirmedBlock::from(versioned_block)
.into_legacy_block()
let confirmed_block = ConfirmedBlock::from(versioned_block);
let block = confirmed_block
.encode_with_options(
UiTransactionEncoding::Json,
BlockEncodingOptions {
transaction_details: TransactionDetails::Signatures,
show_rewards: false,
max_supported_transaction_version: None,
},
)
.unwrap();
let block = legacy_block.configure(
UiTransactionEncoding::Json,
TransactionDetails::Signatures,
false,
);
assert_eq!(actual.value.slot, slot);
assert!(block.eq(&actual.value.block.unwrap()));
}
Expand Down
6 changes: 5 additions & 1 deletion client/src/mock_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use {
pubkey::Pubkey,
signature::Signature,
sysvar::epoch_schedule::EpochSchedule,
transaction::{self, Transaction, TransactionError},
transaction::{self, Transaction, TransactionError, TransactionVersion},
},
solana_transaction_status::{
EncodedConfirmedBlock, EncodedConfirmedTransactionWithStatusMeta, EncodedTransaction,
Expand Down Expand Up @@ -192,6 +192,7 @@ impl RpcSender for MockSender {
"getTransaction" => serde_json::to_value(EncodedConfirmedTransactionWithStatusMeta {
slot: 2,
transaction: EncodedTransactionWithStatusMeta {
version: Some(TransactionVersion::LEGACY),
transaction: EncodedTransaction::Json(
UiTransaction {
signatures: vec!["3AsdoALgZFuq2oUVWrDYhg2pNeaLJKPLf8hU2mQ6U8qJxeJ6hsrPVpMn9ma39DtfYCrDQSvngWRP8NnTpEhezJpE".to_string()],
Expand All @@ -213,6 +214,7 @@ impl RpcSender for MockSender {
accounts: vec![0, 1],
data: "3Bxs49DitAvXtoDR".to_string(),
}],
address_table_lookups: None,
})
}),
meta: Some(UiTransactionStatusMeta {
Expand All @@ -226,6 +228,7 @@ impl RpcSender for MockSender {
pre_token_balances: None,
post_token_balances: None,
rewards: None,
loaded_addresses: None,
}),
},
block_time: Some(1628633791),
Expand Down Expand Up @@ -381,6 +384,7 @@ impl RpcSender for MockSender {
UiTransactionEncoding::Base58,
),
meta: None,
version: Some(TransactionVersion::LEGACY),
}],
rewards: Rewards::new(),
block_time: None,
Expand Down
2 changes: 2 additions & 0 deletions client/src/nonblocking/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2411,6 +2411,7 @@ impl RpcClient {
/// transaction_details: Some(TransactionDetails::None),
/// rewards: Some(true),
/// commitment: None,
/// max_supported_transaction_version: Some(0),
/// };
/// let block = rpc_client.get_block_with_config(
/// slot,
Expand Down Expand Up @@ -3051,6 +3052,7 @@ impl RpcClient {
/// let config = RpcTransactionConfig {
/// encoding: Some(UiTransactionEncoding::Json),
/// commitment: Some(CommitmentConfig::confirmed()),
/// max_supported_transaction_version: Some(0),
/// };
/// let transaction = rpc_client.get_transaction_with_config(
/// &signature,
Expand Down
2 changes: 2 additions & 0 deletions client/src/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,7 @@ impl RpcClient {
/// transaction_details: Some(TransactionDetails::None),
/// rewards: Some(true),
/// commitment: None,
/// max_supported_transaction_version: Some(0),
/// };
/// let block = rpc_client.get_block_with_config(
/// slot,
Expand Down Expand Up @@ -2596,6 +2597,7 @@ impl RpcClient {
/// let config = RpcTransactionConfig {
/// encoding: Some(UiTransactionEncoding::Json),
/// commitment: Some(CommitmentConfig::confirmed()),
/// max_supported_transaction_version: Some(0),
/// };
/// let transaction = rpc_client.get_transaction_with_config(
/// &signature,
Expand Down
3 changes: 3 additions & 0 deletions client/src/rpc_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ pub struct RpcBlockSubscribeConfig {
pub encoding: Option<UiTransactionEncoding>,
pub transaction_details: Option<TransactionDetails>,
pub show_rewards: Option<bool>,
pub max_supported_transaction_version: Option<u8>,
}

#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
Expand Down Expand Up @@ -248,6 +249,7 @@ pub struct RpcBlockConfig {
pub rewards: Option<bool>,
#[serde(flatten)]
pub commitment: Option<CommitmentConfig>,
pub max_supported_transaction_version: Option<u8>,
}

impl EncodingConfig for RpcBlockConfig {
Expand Down Expand Up @@ -288,6 +290,7 @@ pub struct RpcTransactionConfig {
pub encoding: Option<UiTransactionEncoding>,
#[serde(flatten)]
pub commitment: Option<CommitmentConfig>,
pub max_supported_transaction_version: Option<u8>,
}

impl EncodingConfig for RpcTransactionConfig {
Expand Down
17 changes: 14 additions & 3 deletions client/src/rpc_custom_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use {
crate::rpc_response::RpcSimulateTransactionResult,
jsonrpc_core::{Error, ErrorCode},
solana_sdk::clock::Slot,
solana_transaction_status::EncodeError,
thiserror::Error,
};

Expand Down Expand Up @@ -59,7 +60,7 @@ pub enum RpcCustomError {
#[error("BlockStatusNotAvailableYet")]
BlockStatusNotAvailableYet { slot: Slot },
#[error("UnsupportedTransactionVersion")]
UnsupportedTransactionVersion,
UnsupportedTransactionVersion(u8),
}

#[derive(Debug, Serialize, Deserialize)]
Expand All @@ -68,6 +69,16 @@ pub struct NodeUnhealthyErrorData {
pub num_slots_behind: Option<Slot>,
}

impl From<EncodeError> for RpcCustomError {
fn from(err: EncodeError) -> Self {
match err {
EncodeError::UnsupportedTransactionVersion(version) => {
Self::UnsupportedTransactionVersion(version)
}
}
}
}

impl From<RpcCustomError> for Error {
fn from(e: RpcCustomError) -> Self {
match e {
Expand Down Expand Up @@ -172,9 +183,9 @@ impl From<RpcCustomError> for Error {
message: format!("Block status not yet available for slot {}", slot),
data: None,
},
RpcCustomError::UnsupportedTransactionVersion => Self {
RpcCustomError::UnsupportedTransactionVersion(version) => Self {
code: ErrorCode::ServerError(JSON_RPC_SERVER_ERROR_UNSUPPORTED_TRANSACTION_VERSION),
message: "Versioned transactions are not supported".to_string(),
message: format!("Transaction version ({}) is not supported", version),
data: None,
},
}
Expand Down
2 changes: 2 additions & 0 deletions client/src/rpc_deprecated_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ impl From<RpcConfirmedBlockConfig> for RpcBlockConfig {
transaction_details: config.transaction_details,
rewards: config.rewards,
commitment: config.commitment,
max_supported_transaction_version: None,
}
}
}
Expand Down Expand Up @@ -98,6 +99,7 @@ impl From<RpcConfirmedTransactionConfig> for RpcTransactionConfig {
Self {
encoding: config.encoding,
commitment: config.commitment,
max_supported_transaction_version: None,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions client/src/rpc_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ pub enum RpcBlockUpdateError {
#[error("block store error")]
BlockStoreError,

#[error("unsupported transaction version")]
UnsupportedTransactionVersion,
#[error("unsupported transaction version ({0})")]
UnsupportedTransactionVersion(u8),
}

#[derive(Serialize, Deserialize, Debug)]
Expand Down
4 changes: 1 addition & 3 deletions core/src/banking_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3384,9 +3384,7 @@ mod tests {
account_address: Pubkey,
address_lookup_table: AddressLookupTable<'static>,
) -> AccountSharedData {
let mut data = Vec::new();
address_lookup_table.serialize_for_tests(&mut data).unwrap();

let data = address_lookup_table.serialize_for_tests().unwrap();
let mut account =
AccountSharedData::new(1, data.len(), &solana_address_lookup_table_program::id());
account.set_data(data);
Expand Down
13 changes: 8 additions & 5 deletions core/src/replay_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3129,7 +3129,7 @@ pub mod tests {
},
solana_rpc::{
optimistically_confirmed_bank_tracker::OptimisticallyConfirmedBank,
rpc::create_test_transactions_and_populate_blockstore,
rpc::{create_test_transaction_entries, populate_blockstore_for_tests},
},
solana_runtime::{
accounts_background_service::AbsRequestSender,
Expand Down Expand Up @@ -3998,15 +3998,18 @@ pub mod tests {
let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
let slot = bank1.slot();

let mut test_signatures_iter = create_test_transactions_and_populate_blockstore(
let (entries, test_signatures) = create_test_transaction_entries(
vec![&mint_keypair, &keypair1, &keypair2, &keypair3],
bank0.slot(),
bank1.clone(),
);
populate_blockstore_for_tests(
entries,
bank1,
blockstore.clone(),
Arc::new(AtomicU64::default()),
)
.into_iter();
);

let mut test_signatures_iter = test_signatures.into_iter();
let confirmed_block = blockstore.get_rooted_block(slot, false).unwrap();
let actual_tx_results: Vec<_> = confirmed_block
.transactions
Expand Down
16 changes: 15 additions & 1 deletion docs/src/developing/clients/jsonrpc-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ Returns identity and transaction information about a confirmed block in the ledg
- (optional) `transactionDetails: <string>` - level of transaction detail to return, either "full", "signatures", or "none". If parameter not provided, the default detail level is "full".
- (optional) `rewards: bool` - whether to populate the `rewards` array. If parameter not provided, the default includes rewards.
- (optional) [Commitment](jsonrpc-api.md#configuring-state-commitment); "processed" is not supported. If parameter not provided, the default is "finalized".
- (optional) `maxSupportedTransactionVersion: <number>` - set the max transaction version to return in responses. If the requested block contains a transaction with a higher version, an error will be returned.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I might be being dense, but I'm not sure I follow why this is necessary for either getBlock and getTransaction? We generally consider adding fields to rpc responses as non-breaking, and it looks to me like all the new rpc fields are additive (loadedAddresses and version for blocks, addressTableLookups for transactions).

Copy link
Contributor

@CriesofCarrots CriesofCarrots Feb 28, 2022

Choose a reason for hiding this comment

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

Is it that you think it's important for clients to receive an error rather than a v0 block/transaction that is just missing those fields? If so, I don't think I agree. Change my mind?

Copy link
Member Author

Choose a reason for hiding this comment

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

My intent here is that we can totally diverge from keeping backwards compatible fields for future transaction versions if we want.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it that you think it's important for clients to receive an error rather than a v0 block/transaction that is just missing those fields? If so, I don't think I agree. Change my mind?

And yes, I also think this is important. Clients might be parsing transactions for certain account keys and if they don't explicitly support the loaded addresses field for v0 transactions, they can totally miss important certain accounts

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I still don't really love that it forces all clients to upgrade (unexpectedly) when the first v0 transaction comes down the pipe. But to confirm, this won't affect clients querying getBlock with transactionDetails: none|signatures, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, I will start giving this a proper review now :)

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you envision the client upgrade / feature-activation process looking like? Basically every exchange will be affected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I still don't really love that it forces all clients to upgrade (unexpectedly) when the first v0 transaction comes down the pipe

Yeah, I don't love the forced upgrade either but I do feel like this approach opens the door for us to start iterating on the transaction format. We will likely be changing other things about the format in the future and it's inevitable that clients will need to be upgraded to support new features.

What do you envision the client upgrade / feature-activation process looking like? Basically every exchange will be affected.

The rollout process should look like this:

  1. Clients are updated with support for handling versioned transactions
  2. RPC servers and runtime are updated to handle versioned transactions
  3. Once clients and validators are updated, the feature can be activated
  4. Users start processing these new versioned transactions
  5. Old clients will start getting errors when they request blocks / transactions which they don't opt in to receiving
  6. Users complain and realize they need to update their clients

But to confirm, this won't affect clients querying getBlock with transactionDetails: none|signatures, right?

Correct


#### Results:

Expand All @@ -413,6 +414,10 @@ The result field will be an object with the following fields:
- DEPRECATED: `status: <object>` - Transaction status
- `"Ok": <null>` - Transaction was successful
- `"Err": <ERR>` - Transaction failed with TransactionError
- `loadedAddresses: <object|undefined>` - Transaction addresses loaded from address lookup tables. Undefined if `maxSupportedTransactionVersion` is not set in request params.
- `writable: <array[string]>` - Ordered list of base-58 encoded addresses for writable loaded accounts
- `readonly: <array[string]>` - Ordered list of base-58 encoded addresses for readonly loaded accounts
- `version: <"legacy"|number|undefined>` - Transaction version. Undefined if `maxSupportedTransactionVersion` is not set in request params.
- `signatures: <array>` - present if "signatures" are requested for transaction details; an array of signatures strings, corresponding to the transaction order in the block
- `rewards: <array>` - present if rewards are requested; an array of JSON objects containing:
- `pubkey: <string>` - The public key, as base-58 encoded string, of the account that received the reward
Expand Down Expand Up @@ -559,6 +564,10 @@ The JSON structure of a transaction is defined as follows:
- `programIdIndex: <number>` - Index into the `message.accountKeys` array indicating the program account that executes this instruction.
- `accounts: <array[number]>` - List of ordered indices into the `message.accountKeys` array indicating which accounts to pass to the program.
- `data: <string>` - The program input data encoded in a base-58 string.
- `addressTableLookups: <array[object]|undefined>` - List of address table lookups used by a transaction to dynamically load addresses from on-chain address lookup tables. Undefined if `maxSupportedTransactionVersion` is not set.
- `accountKey: <string>` - base-58 encoded public key for an address lookup table account.
- `writableIndexes: <array[number]>` - List of indices used to load addresses of writable accounts from a lookup table.
- `readonlyIndexes: <array[number]>` - List of indices used to load addresses of readonly accounts from a lookup table.

#### Inner Instructions Structure

Expand Down Expand Up @@ -2313,7 +2322,7 @@ Returns the slot leaders for a given slot range

#### Results:

- `<array<string>>` - Node identity public keys as base-58 encoded strings
- `<array[string]>` - Node identity public keys as base-58 encoded strings

#### Example:

Expand Down Expand Up @@ -2847,6 +2856,7 @@ Returns transaction details for a confirmed transaction
- (optional) `encoding: <string>` - encoding for each returned Transaction, either "json", "jsonParsed", "base58" (_slow_), "base64". If parameter not provided, the default encoding is "json".
"jsonParsed" encoding attempts to use program-specific instruction parsers to return more human-readable and explicit data in the `transaction.message.instructions` list. If "jsonParsed" is requested but a parser cannot be found, the instruction falls back to regular JSON encoding (`accounts`, `data`, and `programIdIndex` fields).
- (optional) [Commitment](jsonrpc-api.md#configuring-state-commitment); "processed" is not supported. If parameter not provided, the default is "finalized".
- (optional) `maxSupportedTransactionVersion: <number>` - set the max transaction version to return in responses. If the requested transaction is a higher version, an error will be returned.

#### Results:

Expand All @@ -2873,6 +2883,10 @@ Returns transaction details for a confirmed transaction
- `postBalance: <u64>` - account balance in lamports after the reward was applied
- `rewardType: <string>` - type of reward: currently only "rent", other types may be added in the future
- `commission: <u8|undefined>` - vote account commission when the reward was credited, only present for voting and staking rewards
- `loadedAddresses: <object|undefined>` - Transaction addresses loaded from address lookup tables. Undefined if `maxSupportedTransactionVersion` is not set in request params.
- `writable: <array[string]>` - Ordered list of base-58 encoded addresses for writable loaded accounts
- `readonly: <array[string]>` - Ordered list of base-58 encoded addresses for readonly loaded accounts
- `version: <"legacy"|number|undefined>` - Transaction version. Undefined if `maxSupportedTransactionVersion` is not set in request params.

#### Example:

Expand Down
Loading