Skip to content

Commit

Permalink
Only encode data with <128 bytes in base58 (solana-labs#19317)
Browse files Browse the repository at this point in the history 
* Only encode data with <128 bytes in bs58

* Use same MAX_BS58_BYTES const in rpc

* Fix test

* Spell out base

Co-authored-by: Tyera Eulberg <tyera@solana.com>
  • Loading branch information
@sakridge
sakridge and Tyera Eulberg authored Aug 20, 2021
1 parent 4d361af commit 967746a
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 114 deletions.
27 changes: 20 additions & 7 deletions account-decoder/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use {

pub type StringAmount = String;
pub type StringDecimals = String;
pub const MAX_BASE58_BYTES: usize = 128;

/// A duplicate representation of an Account for pretty JSON serialization
#[derive(Serialize, Deserialize, Clone, Debug)]
Expand Down Expand Up @@ -60,6 +61,17 @@ pub enum UiAccountEncoding {
}

impl UiAccount {
fn encode_bs58<T: ReadableAccount>(
account: &T,
data_slice_config: Option<UiDataSliceConfig>,
) -> String {
if account.data().len() <= MAX_BASE58_BYTES {
bs58::encode(slice_data(account.data(), data_slice_config)).into_string()
} else {
"error: data too large for bs58 encoding".to_string()
}
}

pub fn encode<T: ReadableAccount>(
pubkey: &Pubkey,
account: &T,
Expand All @@ -68,13 +80,14 @@ impl UiAccount {
data_slice_config: Option<UiDataSliceConfig>,
) -> Self {
let data = match encoding {
UiAccountEncoding::Binary => UiAccountData::LegacyBinary(
bs58::encode(slice_data(account.data(), data_slice_config)).into_string(),
),
UiAccountEncoding::Base58 => UiAccountData::Binary(
bs58::encode(slice_data(account.data(), data_slice_config)).into_string(),
encoding,
),
UiAccountEncoding::Binary => {
let data = Self::encode_bs58(account, data_slice_config);
UiAccountData::LegacyBinary(data)
}
UiAccountEncoding::Base58 => {
let data = Self::encode_bs58(account, data_slice_config);
UiAccountData::Binary(data, encoding)
}
UiAccountEncoding::Base64 => UiAccountData::Binary(
base64::encode(slice_data(account.data(), data_slice_config)),
encoding,
Expand Down
6 changes: 3 additions & 3 deletions rpc/src/rpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use {
serde::{Deserialize, Serialize},
solana_account_decoder::{
parse_token::{spl_token_id_v2_0, token_amount_to_ui_amount, UiTokenAmount},
UiAccount, UiAccountEncoding, UiDataSliceConfig,
UiAccount, UiAccountEncoding, UiDataSliceConfig, MAX_BASE58_BYTES,
},
solana_client::{
rpc_cache::LargestAccountsCache,
Expand Down Expand Up @@ -2103,9 +2103,9 @@ fn encode_account<T: ReadableAccount>(
data_slice: Option<UiDataSliceConfig>,
) -> Result<UiAccount> {
if (encoding == UiAccountEncoding::Binary || encoding == UiAccountEncoding::Base58)
&& account.data().len() > 128
&& account.data().len() > MAX_BASE58_BYTES
{
let message = "Encoded binary (base 58) data should be less than 128 bytes, please use Base64 encoding.".to_string();
let message = format!("Encoded binary (base 58) data should be less than {} bytes, please use Base64 encoding.", MAX_BASE58_BYTES);
Err(error::Error {
code: error::ErrorCode::InvalidRequest,
message,
Expand Down
5 changes: 3 additions & 2 deletions rpc/src/rpc_pubsub.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,14 +827,15 @@ mod tests {
max_active_subscriptions: MAX_ACTIVE_SUBSCRIPTIONS,
};
let session = create_session();
let encoding = UiAccountEncoding::Base64;
let (subscriber, _id_receiver, receiver) = Subscriber::new_test("accountNotification");
rpc.account_subscribe(
session,
subscriber,
stake_account.pubkey().to_string(),
Some(RpcAccountInfoConfig {
commitment: Some(CommitmentConfig::processed()),
encoding: None,
encoding: Some(encoding),
data_slice: None,
}),
);
Expand Down Expand Up @@ -873,7 +874,7 @@ mod tests {
"value": {
"owner": stake_program_id.to_string(),
"lamports": 51,
"data": bs58::encode(expected_data).into_string(),
"data": [base64::encode(expected_data), encoding],
"executable": false,
"rentEpoch": 0,
},
Expand Down
186 changes: 84 additions & 102 deletions rpc/src/rpc_subscriptions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,26 @@ pub(crate) mod tests {
inner_receiver.recv().expect("recv error")
}

fn make_account_result(lamports: u64, subscription: u64, data: &str) -> serde_json::Value {
json!({
"jsonrpc": "2.0",
"method": "accountNotification",
"params": {
"result": {
"context": { "slot": 1 },
"value": {
"data": data,
"executable": false,
"lamports": lamports,
"owner": "11111111111111111111111111111111",
"rentEpoch": 0,
},
},
"subscription": subscription,
}
})
}

#[test]
#[serial]
fn test_check_account_subscribe() {
Expand All @@ -1400,12 +1420,6 @@ pub(crate) mod tests {
bank_forks.write().unwrap().insert(bank1);
let alice = Keypair::new();

let (create_sub, _id_receiver, create_recv) = Subscriber::new_test("accountNotification");
let (close_sub, _id_receiver, close_recv) = Subscriber::new_test("accountNotification");

let create_sub_id = SubscriptionId::Number(0);
let close_sub_id = SubscriptionId::Number(1);

let exit = Arc::new(AtomicBool::new(false));
let subscriptions = RpcSubscriptions::new(
&exit,
Expand All @@ -1415,118 +1429,86 @@ pub(crate) mod tests {
))),
OptimisticallyConfirmedBank::locked_from_bank_forks_root(&bank_forks),
);
subscriptions.add_account_subscription(
alice.pubkey(),
Some(RpcAccountInfoConfig {
commitment: Some(CommitmentConfig::processed()),
encoding: None,
data_slice: None,
}),
create_sub_id.clone(),
create_sub,
);

assert!(subscriptions
.subscriptions
.account_subscriptions
.read()
.unwrap()
.contains_key(&alice.pubkey()));

let tx = system_transaction::create_account(
let tx0 = system_transaction::create_account(
&mint_keypair,
&alice,
blockhash,
1,
0,
&system_program::id(),
);
bank_forks
.write()
.unwrap()
.get(1)
.unwrap()
.process_transaction(&tx)
.unwrap();
let commitment_slots = CommitmentSlots {
slot: 1,
..CommitmentSlots::default()
};
subscriptions.notify_subscribers(commitment_slots);
let (response, _) = robust_poll_or_panic(create_recv);
let expected = json!({
"jsonrpc": "2.0",
"method": "accountNotification",
"params": {
"result": {
"context": { "slot": 1 },
"value": {
"data": "",
"executable": false,
"lamports": 1,
"owner": "11111111111111111111111111111111",
"rentEpoch": 0,
},
},
"subscription": 0,
}
});
assert_eq!(serde_json::to_string(&expected).unwrap(), response);
subscriptions.remove_account_subscription(&create_sub_id);

subscriptions.add_account_subscription(
alice.pubkey(),
Some(RpcAccountInfoConfig {
commitment: Some(CommitmentConfig::processed()),
encoding: None,
data_slice: None,
}),
close_sub_id.clone(),
close_sub,
);
let expected0 = make_account_result(1, 0, "");

let tx = {
let tx1 = {
let instruction =
system_instruction::transfer(&alice.pubkey(), &mint_keypair.pubkey(), 1);
let message = Message::new(&[instruction], Some(&mint_keypair.pubkey()));
Transaction::new(&[&alice, &mint_keypair], message, blockhash)
};
let expected1 = make_account_result(0, 1, "");

bank_forks
.write()
.unwrap()
.get(1)
.unwrap()
.process_transaction(&tx)
.unwrap();
subscriptions.notify_subscribers(commitment_slots);
let (response, _) = robust_poll_or_panic(close_recv);
let expected = json!({
"jsonrpc": "2.0",
"method": "accountNotification",
"params": {
"result": {
"context": { "slot": 1 },
"value": {
"data": "",
"executable": false,
"lamports": 0,
"owner": "11111111111111111111111111111111",
"rentEpoch": 0,
},
},
"subscription": 1,
}
});
assert_eq!(serde_json::to_string(&expected).unwrap(), response);
subscriptions.remove_account_subscription(&close_sub_id);
let tx2 = system_transaction::create_account(
&mint_keypair,
&alice,
blockhash,
1,
1024,
&system_program::id(),
);
let expected2 = make_account_result(1, 2, "error: data too large for bs58 encoding");

let subscribe_cases = vec![
(alice.pubkey(), tx0, expected0),
(alice.pubkey(), tx1, expected1),
(alice.pubkey(), tx2, expected2),
];

for (i, (pubkey, tx, expected)) in subscribe_cases.iter().enumerate() {
let (sub, _id_receiver, recv) = Subscriber::new_test("accountNotification");
let sub_id = SubscriptionId::Number(i as u64);
subscriptions.add_account_subscription(
*pubkey,
Some(RpcAccountInfoConfig {
commitment: Some(CommitmentConfig::processed()),
encoding: None,
data_slice: None,
}),
sub_id.clone(),
sub,
);

assert!(!subscriptions
.subscriptions
.account_subscriptions
.read()
.unwrap()
.contains_key(&alice.pubkey()));
assert!(subscriptions
.subscriptions
.account_subscriptions
.read()
.unwrap()
.contains_key(pubkey));

bank_forks
.read()
.unwrap()
.get(1)
.unwrap()
.process_transaction(tx)
.unwrap();
let commitment_slots = CommitmentSlots {
slot: 1,
..CommitmentSlots::default()
};
subscriptions.notify_subscribers(commitment_slots);
let (response, _) = robust_poll_or_panic(recv);

assert_eq!(serde_json::to_string(&expected).unwrap(), response);
subscriptions.remove_account_subscription(&sub_id);

assert!(!subscriptions
.subscriptions
.account_subscriptions
.read()
.unwrap()
.contains_key(pubkey));
}
}

#[test]
Expand Down

0 comments on commit 967746a

Please sign in to comment.