Skip to content

Commit

Permalink
RPC: fix storage_value to return data as read w/o re-encoding in Option
Browse files Browse the repository at this point in the history
  • Loading branch information
@tzemanovic
tzemanovic committed Oct 28, 2022
1 parent e550e12 commit 0f43944
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 34 deletions.
30 changes: 24 additions & 6 deletions apps/src/lib/client/rpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ pub async fn query_raw_bytes(_ctx: Context, args: args::QueryRawBytes) {
.storage_value(&client, None, None, false, &args.storage_key)
.await,
);
match response.data {
Some(bytes) => println!("Found data: 0x{}", HEXLOWER.encode(&bytes)),
None => println!("No data found for key {}", args.storage_key),
if !response.data.is_empty() {
println!("Found data: 0x{}", HEXLOWER.encode(&response.data));
} else {
println!("No data found for key {}", args.storage_key);
}
}

Expand Down Expand Up @@ -1249,17 +1250,34 @@ pub async fn query_storage_value<T>(
where
T: BorshDeserialize,
{
// In case `T` is a unit (only thing that encodes to 0 bytes), we have to
// use `storage_has_key` instead of `storage_value`, because `storage_value`
// returns 0 bytes when the key is not found.
let maybe_unit = T::try_from_slice(&[]);
if let Ok(unit) = maybe_unit {
return if unwrap_client_response(
RPC.shell().storage_has_key(client, key).await,
) {
Some(unit)
} else {
None
};
}

let response = unwrap_client_response(
RPC.shell()
.storage_value(client, None, None, false, key)
.await,
);
response.data.map(|bytes| {
T::try_from_slice(&bytes[..]).unwrap_or_else(|err| {
if response.data.is_empty() {
return None;
}
T::try_from_slice(&response.data[..])
.map(Some)
.unwrap_or_else(|err| {
eprintln!("Error decoding the value: {}", err);
cli::safe_exit(1)
})
})
}

/// Query a range of storage values with a matching prefix and decode them with
Expand Down
85 changes: 74 additions & 11 deletions shared/src/ledger/queries/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,10 @@ macro_rules! handle_match {
break
}
let result = $handle($ctx, $request, $( $matched_args ),* )?;
let data = borsh::BorshSerialize::try_to_vec(&result.data).into_storage_result()?;
return Ok($crate::ledger::queries::EncodedResponseQuery {
data,
info: result.info,
proof_ops: result.proof_ops,
});
// The handle must take care of encoding if needed and return `Vec<u8>`.
// This is because for `storage_value` the bytes are returned verbatim
// as read from storage.
return Ok(result);
};

// Handler function that doesn't use the request, just the path args, if any
Expand All @@ -90,6 +88,7 @@ macro_rules! handle_match {
// If you get a compile error from here with `expected function, found
// queries::Storage`, you're probably missing the marker `(sub _)`
let data = $handle($ctx, $( $matched_args ),* )?;
// Encode the returned data with borsh
let data = borsh::BorshSerialize::try_to_vec(&data).into_storage_result()?;
return Ok($crate::ledger::queries::EncodedResponseQuery {
data,
Expand Down Expand Up @@ -371,6 +370,61 @@ macro_rules! pattern_to_prefix {
/// Turn patterns and their handlers into methods for the router, where each
/// dynamic pattern is turned into a parameter for the method.
macro_rules! pattern_and_handler_to_method {
// Special terminal rule for `storage_value` handle from
// `shared/src/ledger/queries/shell.rs` that returns `Vec<u8>` which should
// not be decoded from response.data, but instead return as is
(
( $( $param:tt: $param_ty:ty ),* )
[ $( { $prefix:expr } ),* ]
$return_type:path,
(with_options storage_value),
()
) => {
// paste! used to construct the `fn $handle_path`'s name.
paste::paste! {
#[allow(dead_code)]
#[doc = "Get a path to query `storage_value`."]
pub fn storage_value_path(&self, $( $param: &$param_ty ),* ) -> String {
itertools::join(
[ Some(std::borrow::Cow::from(&self.prefix)), $( $prefix ),* ]
.into_iter()
.filter_map(|x| x), "/")
}

#[allow(dead_code)]
#[allow(clippy::too_many_arguments)]
#[cfg(any(test, feature = "async-client"))]
#[doc = "Request value with optional data (used for e.g. \
`dry_run_tx`), optionally specified height (supported for \
`storage_value`) and optional proof (supported for \
`storage_value` and `storage_prefix`) from `storage_value`."]
pub async fn storage_value<CLIENT>(&self, client: &CLIENT,
data: Option<Vec<u8>>,
height: Option<$crate::types::storage::BlockHeight>,
prove: bool,
$( $param: &$param_ty ),*
)
-> std::result::Result<
$crate::ledger::queries::ResponseQuery<Vec<u8>>,
<CLIENT as $crate::ledger::queries::Client>::Error
>
where CLIENT: $crate::ledger::queries::Client + std::marker::Sync {
println!("IMMA VEC!!!!!!");
let path = self.storage_value_path( $( $param ),* );

let $crate::ledger::queries::ResponseQuery {
data, info, proof_ops
} = client.request(path, data, height, prove).await?;

Ok($crate::ledger::queries::ResponseQuery {
data,
info,
proof_ops,
})
}
}
};

// terminal rule for $handle that uses request (`with_options`)
(
( $( $param:tt: $param_ty:ty ),* )
Expand Down Expand Up @@ -408,6 +462,7 @@ macro_rules! pattern_and_handler_to_method {
<CLIENT as $crate::ledger::queries::Client>::Error
>
where CLIENT: $crate::ledger::queries::Client + std::marker::Sync {
println!("IMMA not a VEC!!!!!!");
let path = self.[<$handle _path>]( $( $param ),* );

let $crate::ledger::queries::ResponseQuery {
Expand Down Expand Up @@ -681,7 +736,8 @@ macro_rules! router_type {
///
/// // The handler additionally receives the `RequestQuery`, which can have
/// // some data attached, specified block height and ask for a proof. It
/// // returns `ResponseQuery`, which can have some `info` string and a proof.
/// // returns `EncodedResponseQuery` (the `data` must be encoded, if
/// // necessary), which can have some `info` string and a proof.
/// ( "pattern_d" ) -> ReturnType = (with_options handler),
///
/// ( "another" / "pattern" / "that" / "goes" / "deep" ) -> ReturnType = handler,
Expand Down Expand Up @@ -779,9 +835,13 @@ macro_rules! router {
/// ```
#[cfg(test)]
mod test_rpc_handlers {
use crate::ledger::queries::{RequestCtx, RequestQuery, ResponseQuery};
use borsh::BorshSerialize;

use crate::ledger::queries::{
EncodedResponseQuery, RequestCtx, RequestQuery, ResponseQuery,
};
use crate::ledger::storage::{DBIter, StorageHasher, DB};
use crate::ledger::storage_api;
use crate::ledger::storage_api::{self, ResultExt};
use crate::types::storage::Epoch;
use crate::types::token;

Expand Down Expand Up @@ -873,12 +933,12 @@ mod test_rpc_handlers {
pub fn c<D, H>(
_ctx: RequestCtx<'_, D, H>,
_request: &RequestQuery,
) -> storage_api::Result<ResponseQuery<String>>
) -> storage_api::Result<EncodedResponseQuery>
where
D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
H: 'static + StorageHasher + Sync,
{
let data = "c".to_owned();
let data = "c".to_owned().try_to_vec().into_storage_result()?;
Ok(ResponseQuery {
data,
..ResponseQuery::default()
Expand Down Expand Up @@ -1010,6 +1070,9 @@ mod test {
.unwrap();
assert_eq!(result, format!("b3iiii/{a1}/{a2}"));

let result = TEST_RPC.c(&client, None, None, false).await.unwrap();
assert_eq!(result.data, format!("c"));

let result = TEST_RPC.test_sub_rpc().x(&client).await.unwrap();
assert_eq!(result, format!("x"));

Expand Down
42 changes: 25 additions & 17 deletions shared/src/ledger/queries/shell.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use borsh::BorshSerialize;
use tendermint_proto::crypto::{ProofOp, ProofOps};

use crate::ledger::queries::require_latest_height;
use crate::ledger::queries::types::{RequestCtx, RequestQuery, ResponseQuery};
use crate::ledger::queries::types::{RequestCtx, RequestQuery};
use crate::ledger::queries::{require_latest_height, EncodedResponseQuery};
use crate::ledger::storage::{DBIter, StorageHasher, DB};
use crate::ledger::storage_api::{self, ResultExt, StorageRead};
use crate::types::storage::{self, Epoch, PrefixValue};
Expand All @@ -15,7 +16,7 @@ router! {SHELL,

// Raw storage access - read value
( "value" / [storage_key: storage::Key] )
-> Option<Vec<u8>> = (with_options storage_value),
-> Vec<u8> = (with_options storage_value),

// Dry run a transaction
( "dry_run_tx" ) -> TxResult = (with_options dry_run_tx),
Expand All @@ -35,7 +36,7 @@ router! {SHELL,
fn dry_run_tx<D, H>(
mut ctx: RequestCtx<'_, D, H>,
request: &RequestQuery,
) -> storage_api::Result<ResponseQuery<TxResult>>
) -> storage_api::Result<EncodedResponseQuery>
where
D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
H: 'static + StorageHasher + Sync,
Expand All @@ -59,17 +60,19 @@ where
&mut ctx.tx_wasm_cache,
)
.into_storage_result()?;
Ok(ResponseQuery {
let data = data.try_to_vec().into_storage_result()?;
Ok(EncodedResponseQuery {
data,
..ResponseQuery::default()
proof_ops: None,
info: Default::default(),
})
}

#[cfg(not(all(feature = "wasm-runtime", feature = "ferveo-tpke")))]
fn dry_run_tx<D, H>(
_ctx: RequestCtx<'_, D, H>,
_request: &RequestQuery,
) -> storage_api::Result<ResponseQuery<TxResult>>
) -> storage_api::Result<EncodedResponseQuery>
where
D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
H: 'static + StorageHasher + Sync,
Expand All @@ -89,11 +92,15 @@ where
Ok(data)
}

/// Returns data with `vec![]` when the storage key is not found. For all
/// borsh-encoded types, it is safe to check `data.is_empty()` to see if the
/// value was found, except for unit - see `fn query_storage_value` in
/// `apps/src/lib/client/rpc.rs` for unit type handling via `storage_has_key`.
fn storage_value<D, H>(
ctx: RequestCtx<'_, D, H>,
request: &RequestQuery,
storage_key: storage::Key,
) -> storage_api::Result<ResponseQuery<Option<Vec<u8>>>>
) -> storage_api::Result<EncodedResponseQuery>
where
D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
H: 'static + StorageHasher + Sync,
Expand All @@ -117,10 +124,10 @@ where
} else {
None
};
Ok(ResponseQuery {
data: Some(value),
Ok(EncodedResponseQuery {
data: value,
proof_ops: proof,
..Default::default()
info: Default::default(),
})
}
(None, _gas) => {
Expand All @@ -133,8 +140,8 @@ where
} else {
None
};
Ok(ResponseQuery {
data: None,
Ok(EncodedResponseQuery {
data: vec![],
proof_ops: proof,
info: format!("No value found for key: {}", storage_key),
})
Expand All @@ -146,7 +153,7 @@ fn storage_prefix<D, H>(
ctx: RequestCtx<'_, D, H>,
request: &RequestQuery,
storage_key: storage::Key,
) -> storage_api::Result<ResponseQuery<Vec<storage::PrefixValue>>>
) -> storage_api::Result<EncodedResponseQuery>
where
D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
H: 'static + StorageHasher + Sync,
Expand Down Expand Up @@ -177,7 +184,8 @@ where
} else {
None
};
Ok(ResponseQuery {
let data = data.try_to_vec().into_storage_result()?;
Ok(EncodedResponseQuery {
data,
proof_ops,
..Default::default()
Expand Down Expand Up @@ -261,7 +269,7 @@ mod test {
.storage_value(&client, None, None, false, &balance_key)
.await
.unwrap();
assert!(read_balance.data.is_none());
assert!(read_balance.data.is_empty());

// Request storage prefix iterator
let balance_prefix = token::balance_prefix(&token_addr);
Expand Down Expand Up @@ -291,7 +299,7 @@ mod test {
.unwrap();
assert_eq!(
balance,
token::Amount::try_from_slice(&read_balance.data.unwrap()).unwrap()
token::Amount::try_from_slice(&read_balance.data).unwrap()
);

// Request storage prefix iterator
Expand Down

0 comments on commit 0f43944

Please sign in to comment.