darwinia-network / darwinia-common

Darwinia Runtime Pallet Library and Pangolin/Pangoro Testnet
https://rust-docs.darwinia.network/darwinia-common
GNU General Public License v3.0
30 stars 9 forks source link

Ensure `note_min_gas_price_target` updated in a block once #725

Closed hackfisher closed 3 years ago

hackfisher commented 3 years ago
AurevoirXavier commented 3 years ago

For the second question:

https://crates.parity.io/frame_support/inherent/trait.ProvideInherent.html#tymethod.is_inherent https://github.com/paritytech/substrate/blob/master/frame/timestamp/src/lib.rs

boundless-forest commented 3 years ago

Inherent and Unsigned Transaction are both verified by ensure_none?

As far as I understand it, Inherent is a special unsigned transaction. Therefore, you must ensure ensure_none().

For inherent, you can add extra InherentData validation by using check_inherent(). For general unsigned transaction, checked by validate_unsigned().

Does Unsigned Transaction require extra validation for those calls use ensure_none?

Sometimes it is necessary.

With extra validation examples:

  1. https://github.com/darwinia-network/darwinia-common/blob/master/frame/claims/src/lib.rs#L418
  2. https://github.com/darwinia-network/darwinia-common/blob/master/frame/claims/src/lib.rs#L418

No extra validation examples:

  1. https://github.com/paritytech/substrate/blob/master/frame/authorship/src/lib.rs#L220
  2. https://github.com/paritytech/substrate/blob/master/frame/timestamp/src/lib.rs#L190
hackfisher commented 3 years ago

Just as the timestamp pallet from substrate, which validates the timestamp must be updated only once in the block.

Refer:

https://github.com/paritytech/substrate/blob/a019b577163ec354d2776178fbdb922b0e77dea9/frame/timestamp/src/lib.rs#L191

The dvm dynamic fee might be better to do the same check to avoid corner cases of including multiple such note_min_gas_price_target, one from block producer, others might from tx pool, ordered and included by block producer by accident.

https://github.com/darwinia-network/darwinia-common/blob/93f8d4b7b3e9f616ddc0728477bd6347908bd615/frame/dvm-dynamic-fee/src/lib.rs#L79

test

boundless-forest commented 3 years ago

https://substrate.dev/docs/en/knowledgebase/runtime/fees#mandatory-dispatches

boundless-forest commented 3 years ago

https://github.com/paritytech/frontier/pull/435

AurevoirXavier commented 3 years ago

Any update? @AsceticBear

boundless-forest commented 3 years ago

Waiting for the next DVM update

hackfisher commented 3 years ago

others might from tx pool, ordered and included by block producer by accident.

It is not possible to be include by block producer if it uses transaction pool

https://github.com/paritytech/substrate/blob/master/primitives/runtime/src/generic/checked_extrinsic.rs#L51-L66

Inherent does not pass through the transaction pool, so it will not be checked out of NoUnsignedValidator by validate_unsigned, but the Unsigned Extrinsic sent by users outside the block producer will be checked out of the transaction pool and will be checked out of NoUnsignedValidator image (13)

hackfisher commented 3 years ago

Multiple inherent inclusion is fixing by https://github.com/darwinia-network/darwinia-common/pull/817