Skip to content

Commit

Permalink
refactor: style docs and using constants for errors in chain extension
Browse files Browse the repository at this point in the history
  • Loading branch information
@Daanvdplas
Daanvdplas committed Jul 26, 2024
2 parents b50830c + 4e4512e commit 2a8800c
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 57 deletions.
12 changes: 6 additions & 6 deletions pallets/api/src/fungibles/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,18 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
/// Reads the state of the fungible asset based on the provided key.
/// Reads fungible asset state based on the provided value.
///
/// This function matches the key to determine the type of state query and returns the
/// This function matches the value to determine the type of state query and returns the
/// encoded result.
///
/// # Arguments
/// * `key` - An instance of `FungiblesKey<T>`, which specifies the type of state query and
/// # Parameter
/// * `value` - An instance of `Read<T>`, which specifies the type of state query and
/// the associated parameters.
pub fn read_state(key: Read<T>) -> Vec<u8> {
pub fn read_state(value: Read<T>) -> Vec<u8> {
use Read::*;

match key {
match value {
TotalSupply(id) => AssetsOf::<T>::total_supply(id).encode(),
BalanceOf { id, owner } => AssetsOf::<T>::balance(id, owner).encode(),
Allowance { id, owner, spender } => {
Expand Down
65 changes: 33 additions & 32 deletions pop-api/integration-tests/src/local_fungibles.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,25 @@ use pop_primitives::error::{
const ASSET_ID: AssetId = 1;
const CONTRACT: &str = "contracts/fungibles/target/ink/fungibles.wasm";

fn decoded<T: Decode>(result: ExecReturnValue) -> T {
match <T>::decode(&mut &result.data[2..]) {
Ok(value) => value,
Err(_) => panic!("\nTest failed by trying to decode `{:?}` into `T`\n", result),
}
fn decoded<T: Decode>(result: ExecReturnValue) -> Result<T, String> {
<T>::decode(&mut &result.data[2..])
.map_err(|_| format!("\nTest failed by trying to decode `{:?}` into `T`\n", result))
}

// Call total_supply contract message.
fn total_supply(addr: AccountId32, asset_id: AssetId) -> Balance {
let function = function_selector("total_supply");
let params = [function, asset_id.encode()].concat();
let result = bare_call(addr, params, 0).expect("should work");
decoded::<Balance>(result)
decoded::<Balance>(result).unwrap()
}

// Call balance_of contract message.
fn balance_of(addr: AccountId32, asset_id: AssetId, owner: AccountId32) -> Balance {
let function = function_selector("balance_of");
let params = [function, asset_id.encode(), owner.encode()].concat();
let result = bare_call(addr, params, 0).expect("should work");
decoded::<Balance>(result)
decoded::<Balance>(result).unwrap()
}

// Call allowance contract message.
Expand All @@ -41,31 +39,31 @@ fn allowance(
let function = function_selector("allowance");
let params = [function, asset_id.encode(), owner.encode(), spender.encode()].concat();
let result = bare_call(addr, params, 0).expect("should work");
decoded::<Balance>(result)
decoded::<Balance>(result).unwrap()
}

// Call token_name contract message.
fn token_name(addr: AccountId32, asset_id: AssetId) -> Vec<u8> {
let function = function_selector("token_name");
let params = [function, asset_id.encode()].concat();
let result = bare_call(addr, params, 0).expect("should work");
decoded::<Vec<u8>>(result)
decoded::<Vec<u8>>(result).unwrap()
}

// Call token_symbol contract message.
fn token_symbol(addr: AccountId32, asset_id: AssetId) -> Vec<u8> {
let function = function_selector("token_symbol");
let params = [function, asset_id.encode()].concat();
let result = bare_call(addr, params, 0).expect("should work");
decoded::<Vec<u8>>(result)
decoded::<Vec<u8>>(result).unwrap()
}

// Call token_decimals contract message.
fn token_decimals(addr: AccountId32, asset_id: AssetId) -> u8 {
let function = function_selector("token_decimals");
let params = [function, asset_id.encode()].concat();
let result = bare_call(addr, params, 0).expect("should work");
decoded::<u8>(result)
decoded::<u8>(result).unwrap()
}

fn transfer(
Expand Down Expand Up @@ -327,26 +325,26 @@ fn transfer_works() {
// Asset does not exist.
assert_eq!(
decoded::<Error>(transfer(addr.clone(), 1, BOB, amount,)),
Module { index: 52, error: 3 },
Ok(Module { index: 52, error: 3 }),
);
// Create asset with Alice as owner and mint `amount` to contract address.
let asset = create_asset_and_mint_to(ALICE, 1, addr.clone(), amount);
// Asset is not live, i.e. frozen or being destroyed.
freeze_asset(ALICE, asset);
assert_eq!(
decoded::<Error>(transfer(addr.clone(), asset, BOB, amount,)),
Module { index: 52, error: 16 },
Ok(Module { index: 52, error: 16 }),
);
thaw_asset(ALICE, asset);
// Not enough balance.
assert_eq!(
decoded::<Error>(transfer(addr.clone(), asset, BOB, amount + 1 * UNIT)),
Module { index: 52, error: 0 },
Ok(Module { index: 52, error: 0 }),
);
// Not enough balance due to ED.
assert_eq!(
decoded::<Error>(transfer(addr.clone(), asset, BOB, amount)),
Module { index: 52, error: 0 },
Ok(Module { index: 52, error: 0 }),
);
// Successful transfer.
let balance_before_transfer = Assets::balance(asset, &BOB);
Expand All @@ -357,13 +355,13 @@ fn transfer_works() {
// Transfer asset to account that does not exist.
assert_eq!(
decoded::<Error>(transfer(addr.clone(), asset, FERDIE, amount / 4)),
Token(CannotCreate)
Ok(Token(CannotCreate))
);
// Asset is not live, i.e. frozen or being destroyed.
start_destroy_asset(ALICE, asset);
assert_eq!(
decoded::<Error>(transfer(addr.clone(), asset, BOB, amount / 4)),
Module { index: 52, error: 16 },
Ok(Module { index: 52, error: 16 }),
);
});
}
Expand All @@ -377,10 +375,13 @@ fn approve_works() {
// Asset does not exist.
assert_eq!(
decoded::<Error>(approve(addr.clone(), 0, BOB, amount)),
Module { index: 52, error: 3 },
Ok(Module { index: 52, error: 3 }),
);
let asset = create_asset_and_mint_to(ALICE, 0, addr.clone(), amount);
assert_eq!(decoded::<Error>(approve(addr.clone(), asset, BOB, amount)), ConsumerRemaining);
assert_eq!(
decoded::<Error>(approve(addr.clone(), asset, BOB, amount)),
Ok(ConsumerRemaining)
);

let addr = instantiate(CONTRACT, INIT_VALUE, vec![1]);
// Create asset with Alice as owner and mint `amount` to contract address.
Expand All @@ -389,7 +390,7 @@ fn approve_works() {
freeze_asset(ALICE, asset);
assert_eq!(
decoded::<Error>(approve(addr.clone(), asset, BOB, amount)),
Module { index: 52, error: 16 },
Ok(Module { index: 52, error: 16 }),
);
thaw_asset(ALICE, asset);
// Successful approvals:
Expand All @@ -403,7 +404,7 @@ fn approve_works() {
start_destroy_asset(ALICE, asset);
assert_eq!(
decoded::<Error>(approve(addr.clone(), asset, BOB, amount)),
Module { index: 52, error: 16 },
Ok(Module { index: 52, error: 16 }),
);
});
}
Expand All @@ -417,12 +418,12 @@ fn increase_allowance_works() {
// Asset does not exist.
assert_eq!(
decoded::<Error>(increase_allowance(addr.clone(), 0, BOB, amount)),
Module { index: 52, error: 3 },
Ok(Module { index: 52, error: 3 }),
);
let asset = create_asset_and_mint_to(ALICE, 0, addr.clone(), amount);
assert_eq!(
decoded::<Error>(increase_allowance(addr.clone(), asset, BOB, amount)),
ConsumerRemaining
Ok(ConsumerRemaining)
);

let addr = instantiate(CONTRACT, INIT_VALUE, vec![1]);
Expand All @@ -432,7 +433,7 @@ fn increase_allowance_works() {
freeze_asset(ALICE, asset);
assert_eq!(
decoded::<Error>(increase_allowance(addr.clone(), asset, BOB, amount)),
Module { index: 52, error: 16 },
Ok(Module { index: 52, error: 16 }),
);
thaw_asset(ALICE, asset);
// Successful approvals:
Expand All @@ -452,7 +453,7 @@ fn increase_allowance_works() {
start_destroy_asset(ALICE, asset);
assert_eq!(
decoded::<Error>(increase_allowance(addr.clone(), asset, BOB, amount)),
Module { index: 52, error: 16 },
Ok(Module { index: 52, error: 16 }),
);
});
}
Expand Down Expand Up @@ -530,7 +531,7 @@ fn token_metadata_works() {
// // Minting can only be done by the owner.
// assert_eq!(
// decoded::<Error>(transfer_from(addr.clone(), asset, None, Some(BOB), amount, &[0u8])),
// Module { index: 52, error: 2 },
// Ok(Module { index: 52, error: 2 }),
// );
// // Minimum balance of an asset can not be zero.
// assert_eq!(
Expand All @@ -542,7 +543,7 @@ fn token_metadata_works() {
// freeze_asset(addr.clone(), asset);
// assert_eq!(
// decoded::<Error>(transfer_from(addr.clone(), asset, None, Some(BOB), amount, &[0u8])),
// Module { index: 52, error: 16 },
// Ok(Module { index: 52, error: 16 }),
// );
// thaw_asset(addr.clone(), asset);
// // Successful mint.
Expand All @@ -567,7 +568,7 @@ fn token_metadata_works() {
// start_destroy_asset(addr.clone(), asset);
// assert_eq!(
// decoded::<Error>(transfer_from(addr.clone(), asset, None, Some(BOB), amount, &[0u8])),
// Module { index: 52, error: 16 },
// Ok(Module { index: 52, error: 16 }),
// );
// });
// }
Expand All @@ -582,27 +583,27 @@ fn token_metadata_works() {
// // No balance to pay for fees.
// assert_eq!(
// decoded::<Error>(create(addr.clone(), ASSET_ID, addr.clone(), 1)),
// Module { index: 10, error: 2 },
// Ok(Module { index: 10, error: 2 }),
// );
// // Instantiate a contract without balance (relay token).
// let addr = instantiate(CONTRACT, 100, vec![2]);
// // No balance to pay the deposit.
// assert_eq!(
// decoded::<Error>(create(addr.clone(), ASSET_ID, addr.clone(), 1)),
// Module { index: 10, error: 2 },
// Ok(Module { index: 10, error: 2 }),
// );
// // Instantiate a contract with balance.
// let addr =
// instantiate(CONTRACT, INIT_VALUE, vec![1]);
// assert_eq!(
// decoded::<Error>(create(addr.clone(), ASSET_ID, BOB, 0)),
// Module { index: 52, error: 7 },
// Ok(Module { index: 52, error: 7 }),
// );
// create_asset(ALICE, ASSET_ID, 1);
// // Asset ID is already taken.
// assert_eq!(
// decoded::<Error>(create(addr.clone(), ASSET_ID, BOB, 1)),
// Module { index: 52, error: 5 },
// Ok(Module { index: 52, error: 5 }),
// );
// // The minimal balance for an asset must be non zero.
// let new_asset = 2;
Expand Down
47 changes: 28 additions & 19 deletions runtime/devnet/src/extensions/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ use sp_runtime::{traits::Dispatchable, DispatchError};
use sp_std::vec::Vec;

const LOG_TARGET: &str = "pop-api::extension";
const DECODING_FAILED_ERROR: DispatchError = DispatchError::Other("DecodingFailed");
// TODO: issue #93, we can also encode the `pop_primitives::Error::UnknownCall` which means we do use
// `Error` in the runtime and it should stay in primitives. Perhaps issue #91 will also influence
// here. Should be looked at together.
const DECODING_FAILED_ERROR_ENCODED: [u8; 4] = [255u8, 0, 0, 0];
const UNKNOWN_CALL_ERROR: DispatchError = DispatchError::Other("UnknownCall");
// TODO: see above.
const UNKNOWN_CALL_ERROR_ENCODED: [u8; 4] = [254u8, 0, 0, 0];

type ContractSchedule<T> = <T as pallet_contracts::Config>::Schedule;

Expand Down Expand Up @@ -110,8 +118,7 @@ where
params.insert(0, version);
params.insert(1, pallet_index);
params.insert(2, call_index);
let call = <VersionedDispatch>::decode(&mut &params[..])
.map_err(|_| DispatchError::Other("DecodingFailed"))?;
let call = <VersionedDispatch>::decode(&mut &params[..]).map_err(|_| DECODING_FAILED_ERROR)?;

// Contract is the origin by default.
let origin: RuntimeOrigin = RawOrigin::Signed(env.ext().address().clone()).into();
Expand Down Expand Up @@ -171,15 +178,15 @@ where
params.insert(0, version);
params.insert(1, pallet_index);
params.insert(2, call_index);
let key = <VersionedStateRead<T>>::decode(&mut &params[..])
.map_err(|_| DispatchError::Other("DecodingFailed"))?;
let read =
<VersionedStateRead<T>>::decode(&mut &params[..]).map_err(|_| DECODING_FAILED_ERROR)?;

// Charge weight for doing one storage read.
env.charge_weight(T::DbWeight::get().reads(1_u64))?;
let result = match key {
VersionedStateRead::V0(key) => {
ensure!(AllowedApiCalls::contains(&key), DispatchError::Other("UnknownCall"));
match key {
let result = match read {
VersionedStateRead::V0(read) => {
ensure!(AllowedApiCalls::contains(&read), UNKNOWN_CALL_ERROR);
match read {
RuntimeRead::Fungibles(key) => fungibles::Pallet::<T>::read_state(key),
}
},
Expand Down Expand Up @@ -220,16 +227,18 @@ enum VersionedDispatch {
// - `error`: The `DispatchError` encountered during contract execution.
// - `version`: The version of the chain extension, used to determine the known errors.
pub(crate) fn convert_to_status_code(error: DispatchError, version: u8) -> u32 {
// "UnknownCall" and "DecodingFailed" are mapped to specific errors in the API and will
// never change.
let mut encoded_error = match error {
DispatchError::Other("UnknownCall") => Vec::from([254u8, 0, 0, 0]),
DispatchError::Other("DecodingFailed") => Vec::from([255u8, 0, 0, 0]),
_ => error.encode(),
let mut encoded_error: [u8; 4] = match error {
// "UnknownCall" and "DecodingFailed" are mapped to specific errors in the API and will
// never change.
UNKNOWN_CALL_ERROR => UNKNOWN_CALL_ERROR_ENCODED,
DECODING_FAILED_ERROR => DECODING_FAILED_ERROR_ENCODED,
_ => {
let mut encoded_error = error.encode();
// Resize the encoded value to 4 bytes in order to decode the value in a u32 (4 bytes).
encoded_error.resize(4, 0);
encoded_error.try_into().expect("qed, resized to 4 bytes line above")
},
};
// Resize the encoded value to 4 bytes in order to decode the value in a u32 (4 bytes).
encoded_error.resize(4, 0);
let mut encoded_error = encoded_error.try_into().expect("qed, resized to 4 bytes line above");
match version {
// If an unknown variant of the `DispatchError` is detected the error needs to be converted
// into the encoded value of `Error::Other`. This conversion is performed by shifting the bytes one
Expand All @@ -248,7 +257,7 @@ pub(crate) fn convert_to_status_code(error: DispatchError, version: u8) -> u32 {
// - Byte 3:
// - Unused or represents further nested information.
0 => v0::handle_unknown_error(&mut encoded_error),
_ => encoded_error = [254, 0, 0, 0],
_ => encoded_error = UNKNOWN_CALL_ERROR_ENCODED,
}
u32::from_le_bytes(encoded_error)
}
Expand Down Expand Up @@ -280,7 +289,7 @@ impl TryFrom<u8> for FuncId {
0 => Self::Dispatch,
1 => Self::ReadState,
_ => {
return Err(DispatchError::Other("UnknownFuncId"));
return Err(UNKNOWN_CALL_ERROR);
},
};
Ok(id)
Expand Down

0 comments on commit 2a8800c

Please sign in to comment.