Skip to content

Commit

Permalink
[aptos-rosetta] Fix tests, remove panic points
Browse files Browse the repository at this point in the history
To ensure that the system won't panic because there is an
unexpected data type in the stored data
  • Loading branch information
gregnazario committed Sep 13, 2022
1 parent 39d1176 commit 0d36cce
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 52 deletions.
5 changes: 5 additions & 0 deletions crates/aptos-rosetta/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ impl CoinCache {
}

pub fn get_currency_from_cache(&self, coin: &TypeTag) -> Option<Currency> {
// Short circuit for the default coin
if coin == &native_coin_tag() {
return Some(native_coin());
}

let currencies = self.currencies.read().unwrap();
if let Some(currency) = currencies.get(coin) {
currency.clone()
Expand Down
5 changes: 1 addition & 4 deletions crates/aptos-rosetta/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,7 @@ where
let fut = async move {
match handler(request, options).await {
Ok(response) => {
debug!(
"Response: {}",
serde_json::to_string_pretty(&response).unwrap()
);
debug!("Response: {:?}", serde_json::to_string_pretty(&response));
Ok(warp::reply::with_status(
warp::reply::json(&response),
warp::http::StatusCode::OK,
Expand Down
8 changes: 6 additions & 2 deletions crates/aptos-rosetta/src/construction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ async fn construction_metadata(
let signed_transaction = SignedTransaction::new(
unsigned_transaction,
public_key,
Ed25519Signature::try_from([0u8; 64].as_ref()).unwrap(),
Ed25519Signature::try_from([0u8; 64].as_ref())
.expect("Zero signature should always work"),
);

let request = rest_client
Expand Down Expand Up @@ -682,7 +683,10 @@ async fn construction_preprocess(
.as_ref()
.and_then(|inner| inner.max_gas_amount)
.is_none()
&& (public_keys.is_none() || public_keys.unwrap().is_empty())
&& public_keys
.as_ref()
.map(|inner| inner.is_empty())
.unwrap_or(false)
{
return Err(ApiError::InvalidInput(Some(
"Must provide either max gas amount or public keys to estimate max gas amount"
Expand Down
118 changes: 74 additions & 44 deletions crates/aptos-rosetta/src/types/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ impl Transaction {
operation_index += ops.len() as u64;
operations.append(&mut ops);
}
println!("Failed transaction: {:?}", operations)
};

// Reorder operations by type so that there's no invalid ordering
Expand Down Expand Up @@ -571,25 +572,36 @@ fn parse_operations_from_txn_payload(
parse_transfer_from_txn_payload(inner, native_coin(), sender, operation_index)
}
(AccountAddress::ONE, ACCOUNT_MODULE, CREATE_ACCOUNT_FUNCTION) => {
// TODO: Fix unwrap
let address: AccountAddress =
bcs::from_bytes(inner.args().first().unwrap()).unwrap();
operations.push(Operation::create_account(
operation_index,
Some(OperationStatusType::Failure),
address,
sender,
));
if let Some(Ok(address)) = inner
.args()
.get(0)
.map(|encoded| bcs::from_bytes::<AccountAddress>(encoded))
{
operations.push(Operation::create_account(
operation_index,
Some(OperationStatusType::Failure),
address,
sender,
));
} else {
warn!("Failed to parse create account {:?}", inner);
}
}
(AccountAddress::ONE, STAKE_MODULE, SET_OPERATOR_FUNCTION) => {
let operator: AccountAddress =
bcs::from_bytes(inner.args().first().unwrap()).unwrap();
operations.push(Operation::set_operator(
operation_index,
Some(OperationStatusType::Failure),
operator,
sender,
));
if let Some(Ok(operator)) = inner
.args()
.get(0)
.map(|encoded| bcs::from_bytes::<AccountAddress>(encoded))
{
operations.push(Operation::set_operator(
operation_index,
Some(OperationStatusType::Failure),
operator,
sender,
));
} else {
warn!("Failed to parse set operator {:?}", inner);
}
}
_ => {
// If we don't recognize the transaction payload, then we can't parse operations
Expand All @@ -606,22 +618,34 @@ fn parse_transfer_from_txn_payload(
operation_index: u64,
) -> Vec<Operation> {
let mut operations = vec![];
let receiver: AccountAddress = bcs::from_bytes(payload.args().first().unwrap()).unwrap();
let amount: u64 = bcs::from_bytes(payload.args().get(1).unwrap()).unwrap();
operations.push(Operation::withdraw(
operation_index,
Some(OperationStatusType::Failure),
sender,
currency.clone(),
amount,
));
operations.push(Operation::deposit(
operation_index + 1,
Some(OperationStatusType::Failure),
receiver,
currency,
amount,
));

let args = payload.args();
let maybe_receiver = args
.get(0)
.map(|encoded| bcs::from_bytes::<AccountAddress>(encoded));
let maybe_amount = args.get(1).map(|encoded| bcs::from_bytes::<u64>(encoded));

if let (Some(Ok(receiver)), Some(Ok(amount))) = (maybe_receiver, maybe_amount) {
operations.push(Operation::withdraw(
operation_index,
Some(OperationStatusType::Failure),
sender,
currency.clone(),
amount,
));
operations.push(Operation::deposit(
operation_index + 1,
Some(OperationStatusType::Failure),
receiver,
currency,
amount,
));
} else {
warn!(
"Failed to parse account's {} transfer {:?}",
sender, payload
);
}

operations
}
Expand Down Expand Up @@ -675,16 +699,21 @@ async fn parse_operations_from_write_set(
parse_stakepool_changes(version, address, data, events, operation_index)
}
(AccountAddress::ONE, COIN_MODULE, COIN_STORE_RESOURCE, 1) => {
parse_coinstore_changes(
coin_cache,
struct_tag.type_params.first().unwrap().clone(),
version,
address,
data,
events,
operation_index,
)
.await
if let Some(type_tag) = struct_tag.type_params.first() {
parse_coinstore_changes(
coin_cache,
type_tag.clone(),
version,
address,
data,
events,
operation_index,
)
.await
} else {
warn!("Failed to parse coinstore {}", struct_tag);
Ok(operations)
}
}
_ => {
// Any unknown type will just skip the operations
Expand Down Expand Up @@ -762,7 +791,8 @@ async fn parse_coinstore_changes(
events: &[ContractEvent],
mut operation_index: u64,
) -> ApiResult<Vec<Operation>> {
let coin_store: CoinStoreResource = bcs::from_bytes(data).unwrap();
let coin_store: CoinStoreResource =
bcs::from_bytes(data).expect("Must be able to parse coin store");
let mut operations = vec![];

// Retrieve the coin type
Expand Down
5 changes: 3 additions & 2 deletions testsuite/smoke-test/src/rosetta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ async fn test_invalid_transaction_gas_charged() {
.get(txn_version.saturating_sub(block_info.first_version.0) as usize)
.unwrap();

assert_transfer_transaction(
assert_failed_transfer_transaction(
sender,
AccountAddress::from_hex_literal(INVALID_ACCOUNT).unwrap(),
TRANSFER_AMOUNT,
Expand All @@ -1196,7 +1196,7 @@ async fn test_invalid_transaction_gas_charged() {
);
}

fn assert_transfer_transaction(
fn assert_failed_transfer_transaction(
sender: AccountAddress,
receiver: AccountAddress,
transfer_amount: u64,
Expand All @@ -1212,6 +1212,7 @@ fn assert_transfer_transaction(
let rosetta_txn_metadata = &rosetta_txn.metadata;
assert_eq!(TransactionType::User, rosetta_txn_metadata.transaction_type);
assert_eq!(actual_txn.info.version.0, rosetta_txn_metadata.version.0);
// This should have 3, the deposit, withdraw, and fee
assert_eq!(rosetta_txn.operations.len(), 3);

// Check the operations
Expand Down

0 comments on commit 0d36cce

Please sign in to comment.