From 967746abbf1cb6fb1355323eb812373d0a1fc558 Mon Sep 17 00:00:00 2001 From: sakridge Date: Fri, 20 Aug 2021 22:30:59 +0200 Subject: [PATCH] Only encode data with <128 bytes in base58 (#19317) * 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 --- account-decoder/src/lib.rs | 27 +++-- rpc/src/rpc.rs | 6 +- rpc/src/rpc_pubsub.rs | 5 +- rpc/src/rpc_subscriptions.rs | 186 ++++++++++++++++------------------- 4 files changed, 110 insertions(+), 114 deletions(-) diff --git a/account-decoder/src/lib.rs b/account-decoder/src/lib.rs index 904ae3fbd..31cf56e37 100644 --- a/account-decoder/src/lib.rs +++ b/account-decoder/src/lib.rs @@ -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)] @@ -60,6 +61,17 @@ pub enum UiAccountEncoding { } impl UiAccount { + fn encode_bs58( + account: &T, + data_slice_config: Option, + ) -> 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( pubkey: &Pubkey, account: &T, @@ -68,13 +80,14 @@ impl UiAccount { data_slice_config: Option, ) -> 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, diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 36cfe9d57..815c6b9d5 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -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, @@ -2103,9 +2103,9 @@ fn encode_account( data_slice: Option, ) -> Result { 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, diff --git a/rpc/src/rpc_pubsub.rs b/rpc/src/rpc_pubsub.rs index dab2685c9..e7755739b 100644 --- a/rpc/src/rpc_pubsub.rs +++ b/rpc/src/rpc_pubsub.rs @@ -827,6 +827,7 @@ 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, @@ -834,7 +835,7 @@ mod tests { stake_account.pubkey().to_string(), Some(RpcAccountInfoConfig { commitment: Some(CommitmentConfig::processed()), - encoding: None, + encoding: Some(encoding), data_slice: None, }), ); @@ -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, }, diff --git a/rpc/src/rpc_subscriptions.rs b/rpc/src/rpc_subscriptions.rs index 5537cfdc0..055cfc6ad 100644 --- a/rpc/src/rpc_subscriptions.rs +++ b/rpc/src/rpc_subscriptions.rs @@ -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() { @@ -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, @@ -1415,25 +1429,8 @@ 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, @@ -1441,92 +1438,77 @@ pub(crate) mod tests { 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); + let expected0 = make_account_result(1, 0, ""); - subscriptions.add_account_subscription( - alice.pubkey(), - Some(RpcAccountInfoConfig { - commitment: Some(CommitmentConfig::processed()), - encoding: None, - data_slice: None, - }), - close_sub_id.clone(), - close_sub, - ); - - 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"); - assert!(!subscriptions - .subscriptions - .account_subscriptions - .read() - .unwrap() - .contains_key(&alice.pubkey())); + 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(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]