Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Failing to decode the SCALE encoded input in the state_call RPC causes a runtime panic #12548

Closed
Closed
Failing to decode the SCALE encoded input in the state_call RPC causes a runtime panic#12548

Description

@jasl
jasl
opened on Oct 22, 2022

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

When trying upload an Ink contract via https://contracts-ui.substrate.io/add-contract

The node's runtime will panic.

====================

Version: 3.0.0-dev-49734dd1d72

   0: backtrace::capture::Backtrace::new
   1: sp_panic_handler::set::{{closure}}
   2: std::panicking::rust_panic_with_hook
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/panicking.rs:702:17
   3: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/panicking.rs:588:13
   4: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/sys_common/backtrace.rs:138:18
   5: rust_begin_unwind
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/panicking.rs:584:5
   6: core::panicking::panic_fmt
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/core/src/panicking.rs:142:14
   7: kitchensink_runtime::api::dispatch
   8: std::thread::local::LocalKey<T>::with
   9: sc_executor::native_executor::WasmExecutor<H>::with_instance::{{closure}}
  10: sc_executor::wasm_runtime::RuntimeCache::with_instance
  11: <sc_executor::native_executor::NativeElseWasmExecutor<D> as sp_core::traits::CodeExecutor>::call
  12: sp_state_machine::execution::StateMachine<B,H,Exec>::execute_aux
  13: sp_state_machine::execution::StateMachine<B,H,Exec>::execute_using_consensus_failure_handler
  14: <sc_service::client::call_executor::LocalCallExecutor<Block,B,E> as sc_client_api::call_executor::CallExecutor<Block>>::call
  15: <sc_rpc::state::state_full::FullState<BE,Block,Client> as sc_rpc::state::StateBackend<Block,Client>>::call
  16: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll
  17: std::panicking::try
  18: tokio::runtime::task::harness::Harness<T,S>::poll
  19: tokio::runtime::blocking::pool::Inner::run
  20: std::sys_common::backtrace::__rust_begin_short_backtrace
  21: core::ops::function::FnOnce::call_once{{vtable.shim}}
  22: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/alloc/src/boxed.rs:1934:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/alloc/src/boxed.rs:1934:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/sys/unix/thread.rs:108:17
  23: __pthread_deallocate


Thread 'tokio-runtime-worker' panicked at 'Bad input data provided to instantiate: unexpected first byte decoding Option', bin/node/runtime/src/lib.rs:1793

This is a bug. Please report it at:

	https://github.com/paritytech/substrate/issues/new


====================

Version: 3.0.0-dev-49734dd1d72

   0: backtrace::capture::Backtrace::new
   1: sp_panic_handler::set::{{closure}}
   2: std::panicking::rust_panic_with_hook
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/panicking.rs:702:17
   3: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/panicking.rs:588:13
   4: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/sys_common/backtrace.rs:138:18
   5: rust_begin_unwind
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/panicking.rs:584:5
   6: core::panicking::panic_fmt
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/core/src/panicking.rs:142:14
   7: kitchensink_runtime::api::dispatch
   8: std::thread::local::LocalKey<T>::with
   9: sc_executor::native_executor::WasmExecutor<H>::with_instance::{{closure}}
  10: sc_executor::wasm_runtime::RuntimeCache::with_instance
  11: <sc_executor::native_executor::NativeElseWasmExecutor<D> as sp_core::traits::CodeExecutor>::call
  12: sp_state_machine::execution::StateMachine<B,H,Exec>::execute_aux
  13: sp_state_machine::execution::StateMachine<B,H,Exec>::execute_using_consensus_failure_handler
  14: <sc_service::client::call_executor::LocalCallExecutor<Block,B,E> as sc_client_api::call_executor::CallExecutor<Block>>::call
  15: <sc_rpc::state::state_full::FullState<BE,Block,Client> as sc_rpc::state::StateBackend<Block,Client>>::call
  16: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll
  17: std::panicking::try
  18: tokio::runtime::task::harness::Harness<T,S>::poll
  19: tokio::runtime::blocking::pool::Inner::run
  20: std::sys_common::backtrace::__rust_begin_short_backtrace
  21: core::ops::function::FnOnce::call_once{{vtable.shim}}
  22: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/alloc/src/boxed.rs:1934:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/alloc/src/boxed.rs:1934:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/7b46aa594c4bdc507fbd904b6777ca30c37a9209/library/std/src/sys/unix/thread.rs:108:17
  23: __pthread_deallocate


Thread 'tokio-runtime-worker' panicked at 'Bad input data provided to instantiate: unexpected first byte decoding Option', bin/node/runtime/src/lib.rs:1793

This is a bug. Please report it at:

	https://github.com/paritytech/substrate/issues/new

2022-10-23 05:13:40 Evicting failed runtime instance error=Runtime panicked: Bad input data provided to instantiate: unexpected first byte decoding Option
2022-10-23 05:13:40 Evicting failed runtime instance error=Runtime panicked: Bad input data provided to instantiate: unexpected first byte decoding Option

Steps to reproduce

It can be reproduced since polkadot-v0.9.31 and later, may relates to this PR #12429

Activity

  1. jasl commented on Oct 22, 2022

    @jasl
    jasl
    on Oct 22, 2022
    ContributorAuthor

    IMO it looks like ContractsUI hasn't compatible with latest pallet-contracts Runtime API change, so it's pass bad RPC call, but this shouldn't made the Runtime crash

  2. athei commented on Oct 23, 2022

    @athei
    athei
    on Oct 23, 2022
    Member

    I don't think behavior is specific to contracts. It is what happens when the state_call RPC can't decode the arguments. If this is desired behavior is another question.

  3. jasl commented on Oct 23, 2022

    @jasl
    jasl
    on Oct 23, 2022
    ContributorAuthor

    I don't think behavior is specific to contracts. It is what happens when the state_call RPC can't decode the arguments. If this is desired behavior is another question.

    I don't think this relates to recent contracts Runtime API change either.
    The Runtime shouldn't panic when given a bad input, it should just reject it.

  4. athei commented on Oct 23, 2022

    @athei
    athei
    on Oct 23, 2022
    Member

    The title of a issue is a bit of a clickbait, though. Cause the runtime only panics when you have direct access to a node via RPC. And only this one node's runtimes panics. Purely offchain. In this case it might be a bit ugly but not at all dangerous.

    Still, I think it shouldn't be a panic but an error.

  5. jasl commented on Oct 23, 2022

    @jasl
    jasl
    on Oct 23, 2022
    ContributorAuthor

    clickbait

    Do you think it's more clear to rename the title to "Bad Runtime RPC call lead the Runtime panic"?

  6. jasl
    changed the title [-]Contract UI made Substrate node runtime panic[/-] [+]Bad Runtime RPC call lead the Runtime panic[/+] on Oct 23, 2022
  7. athei
    changed the title [-]Bad Runtime RPC call lead the Runtime panic[/-] [+]Failing to decode the SCALE encoded input in the `state_call` causes a runtime panic[/+] on Oct 23, 2022

  8. athei commented on Oct 23, 2022

    @athei
    athei
    on Oct 23, 2022
    Member

    I renamed in a way that I think describes the problem.

  9. athei
    changed the title [-]Failing to decode the SCALE encoded input in the `state_call` causes a runtime panic[/-] [+]Failing to decode the SCALE encoded input in the `state_call` RPC causes a runtime panic[/+] on Oct 23, 2022

  10. bkchr commented on Oct 23, 2022

    @bkchr
    bkchr
    on Oct 23, 2022
    Member

    Panic is completely fine. This is done this way since always and otherwise we would need to wrap return type of a runtime function into an extra result. If you don't follow the Abi, there is no other way than panicking because the input is invalid.

  11. bkchr
    closed this as completedon Oct 23, 2022

  12. jasl commented on Oct 23, 2022

    @jasl
    jasl
    on Oct 23, 2022
    ContributorAuthor

    Panic is completely fine.

    Does it means the node need to recreate one when the Runtime panicked? I'm concerning that could be a DoS vector of node (not the network).

  13. bkchr commented on Oct 24, 2022

    @bkchr
    bkchr
    on Oct 24, 2022
    Member

    No. You need to reset the memory. However, we reset the memory anyway for every new call into the runtime. So, there is no difference to the normal behavior.

  14. jasl commented on Oct 24, 2022

    @jasl
    jasl
    on Oct 24, 2022
    ContributorAuthor

    No. You need to reset the memory. However, we reset the memory anyway for every new call into the runtime. So, there is no difference to the normal behavior.

    So the only issue here is this should be a error or just suppress it, or end-user will confuse this

  15. bkchr commented on Oct 25, 2022

    @bkchr
    bkchr
    on Oct 25, 2022
    Member

    or end-user will confuse this

    Who is the end user here? The node operator?

  16. jasl commented on Oct 25, 2022

    @jasl
    jasl
    on Oct 25, 2022
    ContributorAuthor

    or end-user will confuse this

    Who is the end user here? The node operator?

    Yes, the node operator, including me, when I saw "This is a bug. Please report it at:" my instant thinking is collecting info and submit to here, then you explain to me this is not bug, it's design, but the node said it's a bug, so I guess others would submit this too

    Another old story is #12354
    3 years ago I first met this, I submit to Substrate, I remembered it's you told me that's because DB broken and close the issue, so I know what the problem is.

    But our users (node operators) don't know that, 3 years I got maybe hundreds report (The DB growth faster than we thought) for that issue because the node says it's a bug, please submit to Phala team.

    So I'd proposal it is worthy to suppress expected errors, not raise "Please report it"

  17. bkchr commented on Oct 25, 2022

    @bkchr
    bkchr
    on Oct 25, 2022
    Member

    "This is a bug. Please report it at:"

    Yeah that is because it panicked in the native runtime. A panic in the wasm runtime doesn't print this.

    For the wasm runtime we have the following feature: https://github.com/paritytech/substrate/blob/master/primitives/io/Cargo.toml#L91

    When this feature is enabled, the panics will be returned as an error of the execution and not directly printed to the log.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions