polkadot-evm / frontier

Ethereum compatibility layer for Substrate.
Apache License 2.0
564 stars 471 forks source link

Extrinsic Benchmarks doesn't work and report an `InvalidTransaction::BadProof` error #1365

Open SailorSnoW opened 5 months ago

SailorSnoW commented 5 months ago

Description

Running an extrinsic benchmarks doesn't work and crash with the following error: Error: Client(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid(InvalidTransaction::BadProof))))

After a debug session, it happen at the signature verification of the transaction in the Runtime.

Steps to Reproduce

  1. go into the template folder on this repository.
  2. Run the following command as an example: cargo run --features runtime-benchmarks benchmark extrinsic --pallet=balances --extrinsic=transfer_keep_alive
boundless-forest commented 5 months ago

image

Sounds weird. I added this line as shown in the screenshot and ran the following command in the root of the frontier repo. It works.

~/c/r/frontier (master±) ▶︎︎ ./target/release/frontier-template-node benchmark pallet --chain=dev --steps=50 --repeat=20 --extrinsic=transfer_all --pallet=pallet_balances --output=weights.rs --header=HEADER-APACHE2 --template=./.maintain/frame-weight-template.hbs
Pallet: "pallet_balances", Extrinsic: "transfer_all", Lowest values: [], Highest values: [], Steps: 50, Repeat: 20
Raw Storage Info
========
Storage: `System::Number` (r:1 w:0)
Proof: `System::Number` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`)
Storage: `System::ExecutionPhase` (r:1 w:0)
Proof: `System::ExecutionPhase` (`max_values`: Some(1), `max_size`: Some(5), added: 500, mode: `MaxEncodedLen`)
Storage: `System::EventCount` (r:1 w:1)
Proof: `System::EventCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`)
Storage: `System::Events` (r:1 w:1)
Proof: `System::Events` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
Storage: `System::Account` (r:1 w:0)
Proof: `System::Account` (`max_values`: None, `max_size`: Some(116), added: 2591, mode: `MaxEncodedLen`)

Median Slopes Analysis
========
-- Extrinsic Time --

Model:
Time ~=    35.27
              µs

Reads = 5
Writes = 2
Recorded proof Size = 65

Min Squares Analysis
========
-- Extrinsic Time --

Model:
Time ~=    35.27
              µs

Reads = 5
Writes = 2
Recorded proof Size = 65

Created file: "weights.rs"
2024-04-07 16:44:12 Starting benchmark: pallet_balances::transfer_all 
SailorSnoW commented 5 months ago

isn't the command you executed different from the one I posted tho ? Or cargo run --features runtime-benchmarks benchmark extrinsic is the same that doing cargo run --features runtime-benchmarks benchmark pallet --extrinsic=X ?

I still can't get it to work and it also appear with the RemarkBuilder: cargo run --features runtime-benchmarks benchmark extrinsic --pallet=system --extrinsic=remark

boundless-forest commented 5 months ago

Please have a try based on https://github.com/polkadot-evm/frontier/pull/1380.

I have tried two ways:

According to https://docs.substrate.io/test/benchmark/, it's also recommended to compile first and then run the benchmark command. I have no idea of the extra reason for the differences here yet.

SailorSnoW commented 5 months ago

Based on the PR #1380

The problem still persist for both builders:

RemarkBuilder:

$ ../target/release/frontier-template-node benchmark extrinsic --chain=dev --pallet=system --extrinsic=remark 

2024-04-08 15:04:45 🔨 Initializing Genesis block/state (state: 0x17f3…ad4b, header-hash: 0x15a2…8651)    
2024-04-08 15:04:45 👴 Loading GRANDPA authority set from genesis on what appears to be first startup.    
2024-04-08 15:04:45 Essential task `txpool-background` failed. Shutting down service.    
2024-04-08 15:04:45 Essential task `transaction-pool-task-0` failed. Shutting down service.    
2024-04-08 15:04:45 Essential task `basic-block-import-worker` failed. Shutting down service.    
Error: Client(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid(InvalidTransaction::BadProof))))
2024-04-08 15:04:45 Running 10 warmups...    
2024-04-08 15:04:45 Executing block 100 times    
2024-04-08 15:04:45 Building block, this takes some time...

TransferKeepAliveBuilder:

$ ../target/release/frontier-template-node benchmark extrinsic --chain=dev --pallet=balances --extrinsic=transfer_keep_alive

2024-04-08 15:04:21 🔨 Initializing Genesis block/state (state: 0x17f3…ad4b, header-hash: 0x15a2…8651)    
2024-04-08 15:04:21 👴 Loading GRANDPA authority set from genesis on what appears to be first startup.                                
2024-04-08 15:04:21 Essential task `txpool-background` failed. Shutting down service.    
2024-04-08 15:04:21 Essential task `transaction-pool-task-1` failed. Shutting down service.    
2024-04-08 15:04:21 Essential task `transaction-pool-task-0` failed. Shutting down service.    
2024-04-08 15:04:21 Running 10 warmups...    
2024-04-08 15:04:21 Executing block 100 times    
Error: Client(ApplyExtrinsicFailed(Validity(TransactionValidityError::Invalid(InvalidTransaction::BadProof))))
2024-04-08 15:04:21 Building block, this takes some time...
boundless-forest commented 4 months ago

Have you ever tried using --pallet=frame_system instead of --pallet=system?

SailorSnoW commented 4 months ago

Yes, doesn't work as the name of the pallet seems hard coded inside the builder anyway:

See template/node/src/benchmarking.rs:

/// Generates extrinsics for the `benchmark overhead` command.
///
/// Note: Should only be used for benchmarking.
pub struct RemarkBuilder {
    client: Arc<Client>,
}

impl RemarkBuilder {
    /// Creates a new [`Self`] from the given client.
    pub fn new(client: Arc<Client>) -> Self {
        Self { client }
    }
}

impl frame_benchmarking_cli::ExtrinsicBuilder for RemarkBuilder {
    fn pallet(&self) -> &str {
        "system"
    }

    fn extrinsic(&self) -> &str {
        "remark"
    }

    fn build(&self, nonce: u32) -> std::result::Result<OpaqueExtrinsic, &'static str> {
        let acc = ecdsa::Pair::from_string("//Bob", None).expect("static values are valid; qed");
        let extrinsic: OpaqueExtrinsic = create_benchmark_extrinsic(
            self.client.as_ref(),
            acc,
            SystemCall::remark { remark: vec![] }.into(),
            nonce,
        )
        .into();

        Ok(extrinsic)
    }
}

Also I was checking the Darwinia node source code and I saw that the benchmark Overhead and Extrinsic functionalities was disabled, is there any reason to this or it's just that these benchmarks aren't required for a parachain purpose ?

BenchmarkCmd::Overhead(_) => Err("Unsupported benchmarking command".into()),
BenchmarkCmd::Extrinsic(_) => Err("Unsupported benchmarking command".into()),