Improve Dispatch Errors (#878)

* better dispatch errors

* dry_run to use same DispatchError

* fix dry_run_fails; use correct transfer amount

* Hide ModuleError impl and avoid pulling details from metadata unless user needs them

* fix tests

* actually fix the tests (hopefully..)

* Add a couple more DispatchError test cases

* Add a comment about where the error was copied from

* Also expose a way to obtain the raw module error data

* Remove redundant variant prefixes

* explicit lifetime on From<str> for clarity

* fmt
This commit is contained in:
James Wilson
2023-03-23 09:50:44 +00:00
committed by GitHub
parent 7c252fccf7
commit b5194be5a2
14 changed files with 673 additions and 474 deletions
+112 -46
View File
@@ -10,9 +10,14 @@ use assert_matches::assert_matches;
use codec::{Compact, Decode, Encode};
use frame_metadata::RuntimeMetadataPrefixed;
use sp_core::storage::well_known_keys;
use sp_core::{sr25519::Pair as Sr25519Pair, Pair};
use sp_keyring::AccountKeyring;
use subxt::{
rpc::types::{ChainHeadEvent, FollowEvent, Initialized, RuntimeEvent, RuntimeVersionEvent},
error::{DispatchError, Error, TokenError},
rpc::types::{
ChainHeadEvent, DryRunResult, DryRunResultBytes, FollowEvent, Initialized, RuntimeEvent,
RuntimeVersionEvent,
},
tx::Signer,
utils::AccountId32,
};
@@ -169,8 +174,7 @@ async fn dry_run_passes() {
signed_extrinsic
.dry_run(None)
.await
.expect("dryrunning failed")
.expect("dry run should be successful");
.expect("dryrunning failed");
signed_extrinsic
.submit_and_watch()
@@ -181,49 +185,111 @@ async fn dry_run_passes() {
.unwrap();
}
//// [jsdw] Commented out until Subxt decodes these new Token errors better
// #[tokio::test]
// async fn dry_run_fails() {
// let ctx = test_context().await;
// let api = ctx.client();
//
// wait_for_blocks(&api).await;
//
// let alice = pair_signer(AccountKeyring::Alice.pair());
// let hans = pair_signer(Sr25519Pair::generate().0);
//
// let tx = node_runtime::tx().balances().transfer(
// hans.account_id().clone().into(),
// 100_000_000_000_000_000_000_000_000_000_000_000,
// );
//
// let signed_extrinsic = api
// .tx()
// .create_signed(&tx, &alice, Default::default())
// .await
// .unwrap();
//
// let dry_run_res = signed_extrinsic
// .dry_run(None)
// .await
// .expect("dryrunning failed");
//
// assert_eq!(dry_run_res, Err(DryRunError::DispatchError));
//
// let res = signed_extrinsic
// .submit_and_watch()
// .await
// .unwrap()
// .wait_for_finalized_success()
// .await;
//
// if let Err(subxt::error::Error::Runtime(DispatchError::Module(err))) = res {
// assert_eq!(err.pallet, "Balances");
// assert_eq!(err.error, "InsufficientBalance");
// } else {
// panic!("expected a runtime module error");
// }
// }
#[tokio::test]
async fn dry_run_fails() {
let ctx = test_context().await;
let api = ctx.client();
wait_for_blocks(&api).await;
let alice = pair_signer(AccountKeyring::Alice.pair());
let hans = pair_signer(Sr25519Pair::generate().0);
let tx = node_runtime::tx().balances().transfer(
hans.account_id().clone().into(),
// 7 more than the default amount Alice has, so this should fail; insufficient funds:
1_000_000_000_000_000_000_007,
);
let signed_extrinsic = api
.tx()
.create_signed(&tx, &alice, Default::default())
.await
.unwrap();
let dry_run_res = signed_extrinsic
.dry_run(None)
.await
.expect("dryrunning failed");
assert_eq!(
dry_run_res,
DryRunResult::DispatchError(DispatchError::Token(TokenError::FundsUnavailable))
);
let res = signed_extrinsic
.submit_and_watch()
.await
.unwrap()
.wait_for_finalized_success()
.await;
assert!(
matches!(
res,
Err(Error::Runtime(DispatchError::Token(
TokenError::FundsUnavailable
)))
),
"Expected an insufficient balance, got {res:?}"
);
}
#[tokio::test]
async fn dry_run_result_is_substrate_compatible() {
use sp_runtime::{
transaction_validity::{
InvalidTransaction as SpInvalidTransaction,
TransactionValidityError as SpTransactionValidityError,
},
ApplyExtrinsicResult as SpApplyExtrinsicResult, DispatchError as SpDispatchError,
TokenError as SpTokenError,
};
// We really just connect to a node to get some valid metadata to help us
// decode Dispatch Errors.
let ctx = test_context().await;
let api = ctx.client();
let pairs = vec![
// All ok
(SpApplyExtrinsicResult::Ok(Ok(())), DryRunResult::Success),
// Some transaction error
(
SpApplyExtrinsicResult::Err(SpTransactionValidityError::Invalid(
SpInvalidTransaction::BadProof,
)),
DryRunResult::TransactionValidityError,
),
// Some dispatch errors to check that they decode OK. We've tested module errors
// "in situ" in other places so avoid the complexity of testing them properly here.
(
SpApplyExtrinsicResult::Ok(Err(SpDispatchError::Other("hi"))),
DryRunResult::DispatchError(DispatchError::Other),
),
(
SpApplyExtrinsicResult::Ok(Err(SpDispatchError::CannotLookup)),
DryRunResult::DispatchError(DispatchError::CannotLookup),
),
(
SpApplyExtrinsicResult::Ok(Err(SpDispatchError::BadOrigin)),
DryRunResult::DispatchError(DispatchError::BadOrigin),
),
(
SpApplyExtrinsicResult::Ok(Err(SpDispatchError::Token(SpTokenError::CannotCreate))),
DryRunResult::DispatchError(DispatchError::Token(TokenError::CannotCreate)),
),
];
for (actual, expected) in pairs {
let encoded = actual.encode();
let res = DryRunResultBytes(encoded)
.into_dry_run_result(&api.metadata())
.unwrap();
assert_eq!(res, expected);
}
}
#[tokio::test]
async fn external_signing() {
+47 -41
View File
@@ -7,8 +7,12 @@ use crate::{
pair_signer, test_context,
};
use codec::Decode;
use sp_core::Pair;
use sp_keyring::AccountKeyring;
use subxt::utils::{AccountId32, MultiAddress};
use subxt::{
error::{DispatchError, Error, TokenError},
utils::{AccountId32, MultiAddress},
};
#[tokio::test]
async fn tx_basic_transfer() -> Result<(), subxt::Error> {
@@ -296,46 +300,48 @@ async fn storage_balance_lock() -> Result<(), subxt::Error> {
Ok(())
}
//// [jsdw] Commented out until Subxt decodes these new Token errors better
// #[tokio::test]
// async fn transfer_error() {
// let alice = pair_signer(AccountKeyring::Alice.pair());
// let alice_addr = alice.account_id().clone().into();
// let hans = pair_signer(Pair::generate().0);
// let hans_address = hans.account_id().clone().into();
// let ctx = test_context().await;
// let api = ctx.client();
//
// let to_hans_tx = node_runtime::tx()
// .balances()
// .transfer(hans_address, 100_000_000_000_000_000);
// let to_alice_tx = node_runtime::tx()
// .balances()
// .transfer(alice_addr, 100_000_000_000_000_000);
//
// api.tx()
// .sign_and_submit_then_watch_default(&to_hans_tx, &alice)
// .await
// .unwrap()
// .wait_for_finalized_success()
// .await
// .unwrap();
//
// let res = api
// .tx()
// .sign_and_submit_then_watch_default(&to_alice_tx, &hans)
// .await
// .unwrap()
// .wait_for_finalized_success()
// .await;
//
// if let Err(Error::Runtime(DispatchError::Module(err))) = res {
// assert_eq!(err.pallet, "Balances");
// assert_eq!(err.error, "InsufficientBalance");
// } else {
// panic!("expected a runtime module error");
// }
// }
#[tokio::test]
async fn transfer_error() {
let alice = pair_signer(AccountKeyring::Alice.pair());
let alice_addr = alice.account_id().clone().into();
let hans = pair_signer(Pair::generate().0);
let hans_address = hans.account_id().clone().into();
let ctx = test_context().await;
let api = ctx.client();
let to_hans_tx = node_runtime::tx()
.balances()
.transfer(hans_address, 100_000_000_000_000_000);
let to_alice_tx = node_runtime::tx()
.balances()
.transfer(alice_addr, 100_000_000_000_000_000);
api.tx()
.sign_and_submit_then_watch_default(&to_hans_tx, &alice)
.await
.unwrap()
.wait_for_finalized_success()
.await
.unwrap();
let res = api
.tx()
.sign_and_submit_then_watch_default(&to_alice_tx, &hans)
.await
.unwrap()
.wait_for_finalized_success()
.await;
assert!(
matches!(
res,
Err(Error::Runtime(DispatchError::Token(
TokenError::FundsUnavailable
)))
),
"Expected an insufficient balance, got {res:?}"
);
}
#[tokio::test]
async fn transfer_implicit_subscription() {
+12 -8
View File
@@ -68,8 +68,9 @@ async fn validate_not_possible_for_stash_account() -> Result<(), Error> {
.wait_for_finalized_success()
.await;
assert_matches!(announce_validator, Err(Error::Runtime(DispatchError::Module(err))) => {
assert_eq!(err.pallet, "Staking");
assert_eq!(err.error, "NotController");
let details = err.details().unwrap();
assert_eq!(details.pallet(), "Staking");
assert_eq!(details.error(), "NotController");
});
Ok(())
}
@@ -116,8 +117,9 @@ async fn nominate_not_possible_for_stash_account() -> Result<(), Error> {
.await;
assert_matches!(nomination, Err(Error::Runtime(DispatchError::Module(err))) => {
assert_eq!(err.pallet, "Staking");
assert_eq!(err.error, "NotController");
let details = err.details().unwrap();
assert_eq!(details.pallet(), "Staking");
assert_eq!(details.error(), "NotController");
});
Ok(())
}
@@ -161,8 +163,9 @@ async fn chill_works_for_controller_only() -> Result<(), Error> {
.await;
assert_matches!(chill, Err(Error::Runtime(DispatchError::Module(err))) => {
assert_eq!(err.pallet, "Staking");
assert_eq!(err.error, "NotController");
let details = err.details().unwrap();
assert_eq!(details.pallet(), "Staking");
assert_eq!(details.error(), "NotController");
});
let is_chilled = api
@@ -207,8 +210,9 @@ async fn tx_bond() -> Result<(), Error> {
.await;
assert_matches!(bond_again, Err(Error::Runtime(DispatchError::Module(err))) => {
assert_eq!(err.pallet, "Staking");
assert_eq!(err.error, "AlreadyBonded");
let details = err.details().unwrap();
assert_eq!(details.pallet(), "Staking");
assert_eq!(details.error(), "AlreadyBonded");
});
Ok(())
}