Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

Add DispatchOriginForCallOrigin Config for bridge-dispatch pallet. #100

Closed
hackfisher opened this issue May 31, 2022 · 7 comments · Fixed by #131
Closed

Add DispatchOriginForCallOrigin Config for bridge-dispatch pallet. #100

hackfisher opened this issue May 31, 2022 · 7 comments · Fixed by #131
Assignees

Comments

@hackfisher
Copy link
Contributor

Related darwinia-network/darwinia-common#1235

In this case, we do not need to add substrate_transact then.

something like

fn dispatchOriginForCallOrigin(CallOrigin) -> RawOrigin

For runtime to config, e.g. if the CallOrigin is CallOrigin::SourceAccount(source_account_id) and source_account_id is from derived from ethereum, then calculate derive and dispatch as RawOrigin::EthereumTransaction(n) in Crab/Pangolin runtime.

https://github.com/darwinia-network/darwinia-messages-substrate/blob/main/modules/dispatch/src/lib.rs#L254-L259

Replace the pay_dispatch_fee parameter with Origin here.

&& pay_dispatch_fee(&origin_account, message.weight).is_err()

let origin = RawOrigin::Signed(origin_account).into();

@boundless-forest
Copy link
Member

I see what you mean. I am not so sure if call.dispatch(origin) will perform validate_transaction_in_block inner. Need more investment here.

@hackfisher
Copy link
Contributor Author

I see what you mean. I am not so sure if call.dispatch(origin) will perform validate_transaction_in_block inner. Need more investment here.

It will validate nonce inside Stack Runner:

https://github.com/darwinia-network/darwinia-common/blob/1b44a4734ce4407c831e805c50805b69d3d949bb/frame/dvm/evm/src/runner/stack.rs#L99-L101

@boundless-forest
Copy link
Member

boundless-forest commented Jun 1, 2022

Yeah, some basic checks are done in the stake runner, and some are not. ChainId and gas_limit are not covered in the runner validation. polkadot-evm/frontier#665 might be helpful to us.

fn validate_transaction_common(
    origin: H160,
    transaction_data: &TransactionData,
) -> Result<(U256, u64), TransactionValidityError> {
    println!("this is in the validate transaction in common");
    let gas_limit = transaction_data.gas_limit;

    // We must ensure a transaction can pay the cost of its data bytes.
    // If it can't it should not be included in a block.
    let mut gasometer = evm::gasometer::Gasometer::new(
        gas_limit.low_u64(),
        <T as darwinia_evm::Config>::config(),
    );
    let transaction_cost = match transaction_data.action {
        TransactionAction::Call(_) => evm::gasometer::call_transaction_cost(
            &transaction_data.input,
            &transaction_data.access_list,
        ),
        TransactionAction::Create => evm::gasometer::create_transaction_cost(
            &transaction_data.input,
            &transaction_data.access_list,
        ),
    };
    if gasometer.record_transaction(transaction_cost).is_err() {
        return Err(InvalidTransaction::Custom(
            TransactionValidationError::InvalidGasLimit as u8,
        )
        .into());
    }

    if let Some(chain_id) = transaction_data.chain_id {
        if chain_id != T::ChainId::get() {
            return Err(InvalidTransaction::Custom(
                TransactionValidationError::InvalidChainId as u8,
            )
            .into());
        }
    }

    if gas_limit >= T::BlockGasLimit::get() {
        return Err(InvalidTransaction::Custom(
            TransactionValidationError::InvalidGasLimit as u8,
        )
        .into());
    }

    let base_fee = T::FeeCalculator::min_gas_price();
    let mut priority = 0;
    let gas_price = if let Some(gas_price) = transaction_data.gas_price {
        // Legacy and EIP-2930 transactions.
        // Handle priority here. On legacy transaction everything in gas_price except
        // the current base_fee is considered a tip to the miner and thus the priority.
        priority = gas_price.saturating_sub(base_fee).unique_saturated_into();
        gas_price
    } else if let Some(max_fee_per_gas) = transaction_data.max_fee_per_gas {
        // EIP-1559 transactions.
        max_fee_per_gas
    } else {
        return Err(InvalidTransaction::Payment.into());
    };

    if gas_price < base_fee {
        return Err(InvalidTransaction::Payment.into());
    }

    let mut fee = gas_price.saturating_mul(gas_limit);
    if let Some(max_priority_fee_per_gas) = transaction_data.max_priority_fee_per_gas {
        // EIP-1559 transaction priority is determined by `max_priority_fee_per_gas`.
        // If the transaction do not include this optional parameter, priority is now considered
        // zero.
        priority = max_priority_fee_per_gas.unique_saturated_into();
        // Add the priority tip to the payable fee.
        fee = fee.saturating_add(max_priority_fee_per_gas.saturating_mul(gas_limit));
    }

    let account_data = <T as darwinia_evm::Config>::RingAccountBasic::account_basic(&origin);
    let total_payment = transaction_data.value.saturating_add(fee);
    if account_data.balance < total_payment {
        return Err(InvalidTransaction::Payment.into());
    }
    Ok((account_data.nonce, priority))
}

@boundless-forest
Copy link
Member

Be careful to handle the fee charge part, especially related to the Ethereum::transact(xxx) call.

@hackfisher
Copy link
Contributor Author

For the validate_transaction_common things required for evm.transact call, =

	// Common controls to be performed in the same way by the pool and the
	// State Transition Function (STF).
	// This is the case for all controls except those concerning the nonce.
	fn validate_transaction_common(
		origin: H160,
		transaction_data: &TransactionData,
	) -> Result<(U256, u64), TransactionValidityError> {

We can CallFilter config in bridge-dispatch to validate, because for evm tx, it is not validate in dispatch call, they validate in pool and validate_transaction_in_block

		// filter the call
		if !T::CallFilter::contains(&call) {
			log::trace!(
				target: "runtime::bridge-dispatch",
				"Message {:?}/{:?}: the call ({:?}) is rejected by filter",
				source_chain,
				id,
				call,
			);
			Self::deposit_event(Event::MessageCallRejected(source_chain, id));
			return dispatch_result;
		}

@hackfisher
Copy link
Contributor Author

Another issue is:

For evm tx, the fee is paid inside EVM executor, not in DispatchResultWithPostInfo of outside extrinsic. So if we dispatch to evm.transact it might pay twice, one still in EVM executor using gas_price and gas_limit, another one in pay_dispatch_fee in bridge-dispatch pallet, see #103

	fn apply_validated_transaction(
		source: H160,
		transaction: Transaction,
	) -> DispatchResultWithPostInfo {
		Self::raw_transact(source, transaction.into()).map(|(_, used_gas)| {
			Ok(PostDispatchInfo {
				actual_weight: Some(T::GasWeightMapping::gas_to_weight(
					used_gas.unique_saturated_into(),
				)),
				pays_fee: Pays::No,
			}
			.into())
		})?
	}

@hackfisher
Copy link
Contributor Author

Fixed in #130

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants