Skip to content

Commit

Permalink
Girazoki runtime 901 moonbase alpha approval fix (#974)
Browse files Browse the repository at this point in the history
* Fix approval assets bug plus ts tests (#963)

* pallet-assets fixing error matching

* Add erc20 instance contract

* TS tests with flag to true

* More typescript tests

* prettier

* EditorConfig

* Modify integration tests

* bump spec version

* Remove blake2 contract
  • Loading branch information
girazoki authored Nov 10, 2021
1 parent e351d1b commit b493846
Show file tree
Hide file tree
Showing 7 changed files with 10,831 additions and 23 deletions.
33 changes: 14 additions & 19 deletions precompiles/assets-erc20/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ where
context: &Context,
) -> EvmResult<PrecompileOutput> {
let mut gasometer = Gasometer::new(target_gas);
gasometer.record_cost(RuntimeHelper::<Runtime>::db_write_gas_cost())?;
gasometer.record_log_costs_manual(3, 32)?;

// Parse input.
Expand All @@ -291,40 +290,38 @@ where
Runtime::account_to_asset_id(execution_address)
.ok_or(error("non-assetId address"))?;

let caller: Runtime::AccountId =
Runtime::AddressMapping::into_account_id(context.caller);
let origin = Runtime::AddressMapping::into_account_id(context.caller);

let spender: Runtime::AccountId = Runtime::AddressMapping::into_account_id(spender);

// Dispatch call (if enough gas).
// We first cancel any existing approvals
// Since we cannot check storage, we need to execute this call without knowing whether
// another approval exists already.
// But we know that if no approval exists we should get "Unknown"
// Allowance() should be checked instead of doing this Result matching
let used_gas = match RuntimeHelper::<Runtime>::try_dispatch(
<<Runtime as frame_system::Config>::Call as Dispatchable>::Origin::root(),
pallet_assets::Call::<Runtime, Instance>::force_cancel_approval {
Some(origin.clone()).into(),
pallet_assets::Call::<Runtime, Instance>::cancel_approval {
id: asset_id,
owner: Runtime::Lookup::unlookup(caller),
delegate: Runtime::Lookup::unlookup(spender.clone()),
},
gasometer.remaining_gas()?,
) {
Ok(gas_used) => Ok(gas_used),
Err(ExitError::Other(e)) => {
// One DB read for checking the approval did not exist
if e.contains("Unknown") {
Ok(RuntimeHelper::<Runtime>::db_read_gas_cost())
} else {
Err(ExitError::Other(e))
}
}
// ExitError::Other is only returned if the dispatchable fails with an error
// We cannot format the error in wasm
// In our case, we know that if cancel_approval fails approve_transfer will also
// fail in all cases except the one we are interested on: that it does not exist
// a previous approval. In this case we still want to continue, so it is safe
// to do this
// TODO: Once 0.9.12 is here, we should be able to only call cance_approval if
// such approval exists.
Err(ExitError::Other(_e)) => Ok(2 * RuntimeHelper::<Runtime>::db_read_gas_cost()),
// Any other error can be returned
Err(e) => Err(e),
}?;
gasometer.record_cost(used_gas)?;

let origin = Runtime::AddressMapping::into_account_id(context.caller);

// Dispatch call (if enough gas).
let used_gas = RuntimeHelper::<Runtime>::try_dispatch(
Some(origin).into(),
Expand Down Expand Up @@ -413,8 +410,6 @@ where
context: &Context,
) -> EvmResult<PrecompileOutput> {
let mut gasometer = Gasometer::new(target_gas);
gasometer.record_cost(RuntimeHelper::<Runtime>::db_read_gas_cost())?;
gasometer.record_cost(RuntimeHelper::<Runtime>::db_write_gas_cost())?;
gasometer.record_log_costs_manual(3, 32)?;

// Parse input.
Expand Down
2 changes: 1 addition & 1 deletion precompiles/assets-erc20/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ fn transfer_from_non_incremental_approval() {
Some(Ok(PrecompileOutput {
exit_status: ExitSucceed::Returned,
output: Default::default(),
cost: 115329756u64,
cost: 114357756u64,
logs: LogsBuilder::new(Account::AssetId(0u128).into())
.log3(
SELECTOR_LOG_APPROVAL,
Expand Down
3 changes: 2 additions & 1 deletion runtime/moonbase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("moonbase"),
impl_name: create_runtime_str!("moonbase"),
authoring_version: 3,
spec_version: 0900,
spec_version: 0901,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
Expand Down Expand Up @@ -1382,6 +1382,7 @@ impl Contains<Call> for NormalFilter {
pallet_assets::Call::transfer_keep_alive { .. } => true,
pallet_assets::Call::approve_transfer { .. } => true,
pallet_assets::Call::transfer_approved { .. } => true,
pallet_assets::Call::cancel_approval { .. } => true,
_ => false,
},
_ => true,
Expand Down
4 changes: 2 additions & 2 deletions runtime/moonbase/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ fn asset_erc20_precompiles_approve() {
let expected_result = Some(Ok(PrecompileOutput {
exit_status: ExitSucceed::Returned,
output: Default::default(),
cost: 19035u64,
cost: 16035u64,
logs: LogsBuilder::new(asset_precompile_address)
.log3(
SELECTOR_LOG_APPROVAL,
Expand Down Expand Up @@ -1261,7 +1261,7 @@ fn asset_erc20_precompiles_approve() {
let expected_result = Some(Ok(PrecompileOutput {
exit_status: ExitSucceed::Returned,
output: Default::default(),
cost: 36042u64,
cost: 31042u64,
logs: LogsBuilder::new(asset_precompile_address)
.log3(
SELECTOR_LOG_TRANSFER,
Expand Down
10,269 changes: 10,269 additions & 0 deletions tests/contracts/compiled/ERC20Instance.json

Large diffs are not rendered by default.

133 changes: 133 additions & 0 deletions tests/contracts/sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -839,4 +839,137 @@ export const contractSources: { [key: string]: string } = {
);
}
}`,
ERC20Instance: `
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity ^0.8.0;
/**
* @title ERC20 interface
* @dev see https://github.com/ethereum/EIPs/issues/20
* @dev copied from https://github.com/OpenZeppelin/openzeppelin-contracts
*/
interface IERC20 {
/**
* @dev Total number of tokens in existence
* Selector: 18160ddd
*/
function totalSupply() external view returns (uint256);
/**
* @dev Gets the balance of the specified address.
* Selector: 70a08231
* @param who The address to query the balance of.
* @return An uint256 representing the amount owned by the passed address.
*/
function balanceOf(address who) external view returns (uint256);
/**
* @dev Function to check the amount of tokens that an owner allowed to a spender.
* Selector: dd62ed3e
* @param owner address The address which owns the funds.
* @param spender address The address which will spend the funds.
* @return A uint256 specifying the amount of tokens still available for the spender.
*/
function allowance(address owner, address spender)
external view returns (uint256);
/**
* @dev Transfer token for a specified address
* Selector: a9059cbb
* @param to The address to transfer to.
* @param value The amount to be transferred.
*/
function transfer(address to, uint256 value) external returns (bool);
/**
* @dev Approve the passed address to spend the specified amount of tokens on behalf
* of msg.sender.
* Beware that changing an allowance with this method brings the risk that someone may
* use both the old
* and the new allowance by unfortunate transaction ordering. One possible solution to
* mitigate this race condition is to first reduce the spender's allowance to 0 and set
* the desired value afterwards:
* https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
* Selector: 095ea7b3
* @param spender The address which will spend the funds.
* @param value The amount of tokens to be spent.
*/
function approve(address spender, uint256 value)
external returns (bool);
/**
* @dev Transfer tokens from one address to another
* Selector: 23b872dd
* @param from address The address which you want to send tokens from
* @param to address The address which you want to transfer to
* @param value uint256 the amount of tokens to be transferred
*/
function transferFrom(address from, address to, uint256 value)
external returns (bool);
/**
* @dev Event emited when a transfer has been performed.
* Selector: ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef
* @param from address The address sending the tokens
* @param to address The address receiving the tokens.
* @param value uint256 The amount of tokens transfered.
*/
event Transfer(
address indexed from,
address indexed to,
uint256 value
);
/**
* @dev Event emited when an approval has been registered.
* Selector: 8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925
* @param owner address Owner of the tokens.
* @param spender address Allowed spender.
* @param value uint256 Amount of tokens approved.
*/
event Approval(
address indexed owner,
address indexed spender,
uint256 value
);
}
contract ERC20Instance is IERC20 {
/// The ierc20 at the known pre-compile address.
IERC20 public erc20 = IERC20(0x0000000000000000000000000000000000000802);
function totalSupply() override external view returns (uint256){
// We nominate our target collator with all the tokens provided
return erc20.totalSupply();
}
function balanceOf(address who) override external view returns (uint256){
// We nominate our target collator with all the tokens provided
return erc20.balanceOf(who);
}
function allowance(
address owner,
address spender
) override external view returns (uint256){
return erc20.allowance(owner, spender);
}
function transfer(address to, uint256 value) override external returns (bool) {
return erc20.transfer(to, value);
}
function approve(address spender, uint256 value) override external returns (bool) {
return erc20.transfer(spender, value);
}
function transferFrom(
address from,
address to,
int256 value
) override external returns (bool) {
return erc20.transferFrom(from, to, value);
}
}`,
};
Loading

0 comments on commit b493846

Please sign in to comment.