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

rpc: rework binary encoding. BREAKING CHANGE #11646

Merged
merged 5 commits into from
Aug 18, 2020
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
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.

33 changes: 21 additions & 12 deletions account-decoder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,18 @@ pub struct UiAccount {
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", untagged)]
pub enum UiAccountData {
Binary(String),
LegacyBinary(String), // Legacy. Retained for RPC backwards compatibility
Json(ParsedAccount),
Binary64(String),
Binary(String, UiAccountEncoding),
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
#[serde(rename_all = "camelCase")]
pub enum UiAccountEncoding {
Binary, // SLOW! Avoid this encoding
Binary, // Legacy. Retained for RPC backwards compatibility
Copy link
Member Author

Choose a reason for hiding this comment

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

One day we can remove "binary" when it starts getting in the way. It's no longer documented

Base58,
Base64,
JsonParsed,
Binary64,
}

impl UiAccount {
Expand All @@ -54,20 +55,24 @@ impl UiAccount {
data_slice_config: Option<UiDataSliceConfig>,
) -> Self {
let data = match encoding {
UiAccountEncoding::Binary => UiAccountData::Binary(
UiAccountEncoding::Binary => UiAccountData::LegacyBinary(
bs58::encode(slice_data(&account.data, data_slice_config)).into_string(),
),
UiAccountEncoding::Binary64 => UiAccountData::Binary64(base64::encode(slice_data(
&account.data,
data_slice_config,
))),
UiAccountEncoding::Base58 => UiAccountData::Binary(
bs58::encode(slice_data(&account.data, data_slice_config)).into_string(),
encoding,
),
UiAccountEncoding::Base64 => UiAccountData::Binary(
base64::encode(slice_data(&account.data, data_slice_config)),
encoding,
),
UiAccountEncoding::JsonParsed => {
if let Ok(parsed_data) =
parse_account_data(pubkey, &account.owner, &account.data, additional_data)
{
UiAccountData::Json(parsed_data)
} else {
UiAccountData::Binary64(base64::encode(&account.data))
UiAccountData::Binary(base64::encode(&account.data), UiAccountEncoding::Base64)
}
}
};
Expand All @@ -83,8 +88,12 @@ impl UiAccount {
pub fn decode(&self) -> Option<Account> {
let data = match &self.data {
UiAccountData::Json(_) => None,
UiAccountData::Binary(blob) => bs58::decode(blob).into_vec().ok(),
UiAccountData::Binary64(blob) => base64::decode(blob).ok(),
UiAccountData::LegacyBinary(blob) => bs58::decode(blob).into_vec().ok(),
UiAccountData::Binary(blob, encoding) => match encoding {
UiAccountEncoding::Base58 => bs58::decode(blob).into_vec().ok(),
UiAccountEncoding::Base64 => base64::decode(blob).ok(),
UiAccountEncoding::Binary | UiAccountEncoding::JsonParsed => None,
},
}?;
Some(Account {
lamports: self.lamports,
Expand Down
33 changes: 24 additions & 9 deletions cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
validator_info::*,
vote::*,
};
use clap::{App, AppSettings, Arg, ArgMatches, SubCommand};
use clap::{value_t_or_exit, App, AppSettings, Arg, ArgMatches, SubCommand};
use log::*;
use num_traits::FromPrimitive;
use serde_json::{self, json, Value};
Expand Down Expand Up @@ -821,9 +821,14 @@ pub fn parse_command(
_ => Err(CliError::BadParameter("Invalid signature".to_string())),
},
("decode-transaction", Some(matches)) => {
let encoded_transaction = EncodedTransaction::Binary(
matches.value_of("base58_transaction").unwrap().to_string(),
);
let blob = value_t_or_exit!(matches, "transaction", String);
let encoding = match matches.value_of("encoding").unwrap() {
"base58" => UiTransactionEncoding::Binary,
"base64" => UiTransactionEncoding::Base64,
_ => unreachable!(),
};

let encoded_transaction = EncodedTransaction::Binary(blob, encoding);
if let Some(transaction) = encoded_transaction.decode() {
Ok(CliCommandInfo {
command: CliCommand::DecodeTransaction(transaction),
Expand Down Expand Up @@ -1068,7 +1073,7 @@ fn process_confirm(
if let Some(transaction_status) = status {
if config.verbose {
match rpc_client
.get_confirmed_transaction(signature, UiTransactionEncoding::Binary)
.get_confirmed_transaction(signature, UiTransactionEncoding::Base64)
{
Ok(confirmed_transaction) => {
println!(
Expand Down Expand Up @@ -1124,7 +1129,7 @@ fn process_show_account(
account: UiAccount::encode(
account_pubkey,
account,
UiAccountEncoding::Binary64,
UiAccountEncoding::Base64,
None,
None,
),
Expand Down Expand Up @@ -2155,12 +2160,22 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, '
SubCommand::with_name("decode-transaction")
.about("Decode a base-58 binary transaction")
.arg(
Arg::with_name("base58_transaction")
Arg::with_name("transaction")
.index(1)
.value_name("BASE58_TRANSACTION")
.value_name("TRANSACTION")
.takes_value(true)
.required(true)
.help("transaction to decode"),
)
.arg(
Arg::with_name("encoding")
.index(2)
.value_name("ENCODING")
.possible_values(&["base58", "base64"]) // Subset of `UiTransactionEncoding` enum
.default_value("base58")
.takes_value(true)
.required(true)
.help("The transaction to decode"),
.help("transaction encoding"),
),
)
.subcommand(
Expand Down
2 changes: 1 addition & 1 deletion cli/src/offline/blockhash_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ mod tests {
let rpc_nonce_account = UiAccount::encode(
&nonce_pubkey,
nonce_account,
UiAccountEncoding::Binary64,
UiAccountEncoding::Base64,
None,
None,
);
Expand Down
20 changes: 3 additions & 17 deletions client/src/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@ use bincode::serialize;
use indicatif::{ProgressBar, ProgressStyle};
use log::*;
use serde_json::{json, Value};
use solana_account_decoder::{
parse_token::UiTokenAmount,
UiAccount,
UiAccountData::{Binary, Binary64},
UiAccountEncoding,
};
use solana_account_decoder::{parse_token::UiTokenAmount, UiAccount, UiAccountEncoding};
use solana_sdk::{
account::Account,
clock::{
Expand Down Expand Up @@ -469,7 +464,7 @@ impl RpcClient {
commitment_config: CommitmentConfig,
) -> RpcResult<Option<Account>> {
let config = RpcAccountInfoConfig {
encoding: Some(UiAccountEncoding::Binary64),
encoding: Some(UiAccountEncoding::Base64),
commitment: Some(commitment_config),
data_slice: None,
};
Expand All @@ -487,17 +482,8 @@ impl RpcClient {
}
let Response {
context,
value: mut rpc_account,
value: rpc_account,
} = serde_json::from_value::<Response<Option<UiAccount>>>(result_json)?;
if let Some(ref mut account) = rpc_account {
if let Binary(_) = &account.data {
let tmp = Binary64(String::new());
match std::mem::replace(&mut account.data, tmp) {
Binary(new_data) => account.data = Binary64(new_data),
_ => panic!("should have gotten binary here."),
}
}
}
Comment on lines -492 to -500
Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with Binary(String) and Binary64(String) is visible here. Serde can't distinguish between the two on deserialization with the the untagged attribute, so this workaround was previously needed for Rust RPC clients.

trace!("Response account {:?} {:?}", pubkey, rpc_account);
let account = rpc_account.and_then(|rpc_account| rpc_account.decode());
Ok(Response {
Expand Down
36 changes: 21 additions & 15 deletions core/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,11 @@ impl JsonRpcRequestProcessor {
if let Some(account) = bank.get_account(pubkey) {
if account.owner == spl_token_id_v1_0() && encoding == UiAccountEncoding::JsonParsed {
response = Some(get_parsed_token_account(bank.clone(), pubkey, account));
} else if encoding == UiAccountEncoding::Binary && account.data.len() > 128 {
let message = "Encoded binary (base 58) data should be less than 128 bytes, please use Binary64 encoding.".to_string();
} else if (encoding == UiAccountEncoding::Binary
|| encoding == UiAccountEncoding::Base58)
&& account.data.len() > 128
{
let message = "Encoded binary (base 58) data should be less than 128 bytes, please use Base64 encoding.".to_string();
return Err(error::Error {
code: error::ErrorCode::InvalidRequest,
message,
Expand Down Expand Up @@ -1266,7 +1269,7 @@ fn check_slice_and_encoding(encoding: &UiAccountEncoding, data_slice_is_some: bo
UiAccountEncoding::JsonParsed => {
if data_slice_is_some {
let message =
"Sliced account data can only be encoded using binary (base 58) or binary64 encoding."
"Sliced account data can only be encoded using binary (base 58) or base64 encoding."
.to_string();
Err(error::Error {
code: error::ErrorCode::InvalidRequest,
Expand All @@ -1277,7 +1280,7 @@ fn check_slice_and_encoding(encoding: &UiAccountEncoding, data_slice_is_some: bo
Ok(())
}
}
UiAccountEncoding::Binary | UiAccountEncoding::Binary64 => Ok(()),
UiAccountEncoding::Binary | UiAccountEncoding::Base58 | UiAccountEncoding::Base64 => Ok(()),
}
}

Expand Down Expand Up @@ -3097,13 +3100,13 @@ pub mod tests {
"result": {
"context":{"slot":0},
"value":{
"owner": "11111111111111111111111111111111",
"lamports": 20,
"data": "",
"executable": false,
"rentEpoch": 0
},
"owner": "11111111111111111111111111111111",
"lamports": 20,
"data": "",
"executable": false,
"rentEpoch": 0
},
},
"id": 1,
});
let expected: Response =
Expand All @@ -3119,24 +3122,27 @@ pub mod tests {
bank.store_account(&address, &account);

let req = format!(
r#"{{"jsonrpc":"2.0","id":1,"method":"getAccountInfo","params":["{}", {{"encoding":"binary64"}}]}}"#,
r#"{{"jsonrpc":"2.0","id":1,"method":"getAccountInfo","params":["{}", {{"encoding":"base64"}}]}}"#,
Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, this feels so much nicer. "binary64"? What's that! "base64"? Oh, that's base64.

Copy link
Member

Choose a reason for hiding this comment

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

Yes yes yes!! 🙌

address
);
let res = io.handle_request_sync(&req, meta.clone());
let result: Value = serde_json::from_str(&res.expect("actual response"))
.expect("actual response deserialization");
assert_eq!(result["result"]["value"]["data"], base64::encode(&data));
assert_eq!(
result["result"]["value"]["data"],
json!([base64::encode(&data), "base64"]),
);

let req = format!(
r#"{{"jsonrpc":"2.0","id":1,"method":"getAccountInfo","params":["{}", {{"encoding":"binary64", "dataSlice": {{"length": 2, "offset": 1}}}}]}}"#,
r#"{{"jsonrpc":"2.0","id":1,"method":"getAccountInfo","params":["{}", {{"encoding":"base64", "dataSlice": {{"length": 2, "offset": 1}}}}]}}"#,
address
);
let res = io.handle_request_sync(&req, meta.clone());
let result: Value = serde_json::from_str(&res.expect("actual response"))
.expect("actual response deserialization");
assert_eq!(
result["result"]["value"]["data"],
base64::encode(&data[1..3]),
json!([base64::encode(&data[1..3]), "base64"]),
);

let req = format!(
Expand Down Expand Up @@ -4315,7 +4321,7 @@ pub mod tests {
for TransactionWithStatusMeta { transaction, meta } in
confirmed_block.transactions.into_iter()
{
if let EncodedTransaction::Binary(transaction) = transaction {
if let EncodedTransaction::LegacyBinary(transaction) = transaction {
let decoded_transaction: Transaction =
deserialize(&bs58::decode(&transaction).into_vec().unwrap()).unwrap();
if decoded_transaction.signatures[0] == confirmed_block_signatures[0] {
Expand Down
2 changes: 1 addition & 1 deletion core/tests/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn test_rpc_send_tx() {
use solana_account_decoder::UiAccountEncoding;
use solana_client::rpc_config::RpcAccountInfoConfig;
let config = RpcAccountInfoConfig {
encoding: Some(UiAccountEncoding::Binary64),
encoding: Some(UiAccountEncoding::Base64),
commitment: None,
data_slice: None,
};
Expand Down
Loading