paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

subkey sign-transaction = panicked `Option::unwrap()` on a `None` value' #5180

Closed drandreaskrueger closed 1 year ago

drandreaskrueger commented 4 years ago



Short story:

First see the "Long story short" in this comment below: https://github.com/paritytech/substrate/issues/5180#issuecomment-597129861




Long story:

Signing a transaction fails:

subkey sign-transaction --call "0x200400011074657374" --nonce 0 \
--prior-block-hash 0x1e8e7c41bae60c3611d5905b7c405e46e8eb7ee3b49c8cfb620702069bbeb54c  \
--suri "//Alice" --password ""

The result is

Using a genesis hash of dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40

I have tried many other things, e.g. chose a different block hash, deleted all 0x everywhere, etc.

And tried instead of

--nonce 0 --> --nonce 1
--suri "//Alice" --> --suri "observe analyst fabric sentence burden injury inmate wheat flash labor save antenna"

or

--call "0x200400011074657374" --> --call "0xa8040400ffca08b4e0f53054628a43912ffbb6d00a0362921ba9781932297edc037d197a5d070010a5d4e8"

because the latter is the payload generated by the polkascan library compose_call() - see below.


the backtrace is

RUST_BACKTRACE=1 subkey sign-transaction --call "0x200400011074657374" --nonce 0 \
--prior-block-hash 0x1e8e7c41bae60c3611d5905b7c405e46e8eb7ee3b49c8cfb620702069bbeb54c  \
--suri "//Alice" --password ""

Using a genesis hash of dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:84
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:61
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1025
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1426
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:65
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:50
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:193
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:210
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:471
  11: rust_begin_unwind
             at src/libstd/panicking.rs:375
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:84
  13: core::panicking::panic
             at src/libcore/panicking.rs:51
  14: subkey::main
  15: std::rt::lang_start::{{closure}}
  16: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  17: std::panicking::try::do_call
             at src/libstd/panicking.rs:292
  18: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:78
  19: std::panicking::try
             at src/libstd/panicking.rs:270
  20: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  21: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  22: main
  23: __libc_start_main
  24: _start

The subkey command should really tell us its code commit (and not only v2.0.0), but I am almost sure it is from this commit:

subkey --version; substrate --version
subkey 2.0.0
substrate 2.0.0-3e651110a-x86_64-linux-gnu

which is the official pre-v2.0-3e65111 "Current Stable Version" at https://substrate.dev/en/versions


The new and excellent polkascan substrate library does not YET allow to SIGN-AND-SEND-A-TRANSACTION (that is why I am so desperately fiddling with subkey at all) but it provides a

payload = SubstrateInterface.compose_call()

which I hoped would create the right payload that is then inserted into the --call payload argument of subkey - right?


I was wondering whether this would be the right way:

value,dest = 1000000000000, "0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48"

call = polkascan_substrate.compose_call('Balances', 'transfer',{'dest': dest,'value': value})

signed_tx = sign_with_subkey(call, suri, nonce, chain_tip_hash) # see above

polkascan_substrate.rpc_request(method="author_submitExtrinsic", params=[signed_tx])

?

(Once you fixed the above subkey panick) ... would that scheme be the right way to send transactions? Or what am I missing ?


I am also open to ANY other tutorials and suggestions how to send a signed transaction in Python.

(And PLEASE add such manual pages for us Pythonians, anyways. Thanks.)


Thanks a lot!

drandreaskrueger commented 4 years ago

the sign-transaction subcommand is the "missing chapter" in https://substrate.dev/docs/en/ecosystem/subkey#signing-and-verifying-messages

drandreaskrueger commented 4 years ago

P.S.: The --call "0x200400011074657374" I got from here: https://stackoverflow.com/a/60412278

drandreaskrueger commented 4 years ago

As your default answer would probably be "just checkout master", I have now done that already:

mv ~/.cargo/bin/subkey ~/.cargo/bin/subkey-2.0.0-stable

git checkout a439a7aa5a9a
nice cargo install --force --path ./bin/utils/subkey subkey

cd ~/.cargo/bin
mv subkey subkey-2.0.0-alpha.3-a439a7aa5a9a3 
ln -s subkey-2.0.0-alpha.3-a439a7aa5a9a3 subkey

subkey --version

subkey 2.0.0-alpha.3

but ... the bug (or lack of manual PEBKAC user error) remains:

subkey sign-transaction --call "0x200400011074657374" --nonce 42 --prior-block-hash 0x1e8e7c41bae60c3611d5905b7c405e46e8eb7ee3b49c8cfb620702069bbeb54c  --suri "//Alice" --password ""
Using a genesis hash of dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40
arjanz commented 4 years ago

I'm experiencing the same issue, it seems there is an issue with the --password flag. It's mandatory but when trying to set an empty password it results in a RUST panic.

drandreaskrueger commented 4 years ago

Thanks a lot.

I have just tried --password "a"instead of an empty password but:

subkey sign-transaction --call "0x200400011074657374" --nonce 0 --prior-block-hash 0x1e8e7c41bae60c3611d5905b7c405e46e8eb7ee3b49c8cfb620702069bbeb54c  --suri "//Alice" --password "a"

Using a genesis hash of dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40
drandreaskrueger commented 4 years ago

I would like to know an OLD commit when subkey sign-transaction did NOT have this bug.

There must have been a day on which it had worked, right?

Thank you

drandreaskrueger commented 4 years ago

Or, perhaps an old RUST version on which it compiled without that bug?

When was subkey sign-transaction introduced and tested? Perhaps I can go back to that rust version?

drandreaskrueger commented 4 years ago

I also asked a question in the Riot group 'Substrate Technical'

Thanks a lot. I seem to always forget that; not a chat type of guy.

Interesting, am I only the second person (on the googleplanet) who is actually trying to use subkey sign-transaction for a real problem ? Only 6 hits. Is really everyone else coding in Javascript or Rust now?

Anyways, idea: why not include all subcommands of subkey into the cargo test --release --all routine somehow? Or into some kind of integration testing? Or perhaps into the continuous integration testing on gitlab? Thanks.

drandreaskrueger commented 4 years ago

YESSS. That "dirty hack" gave me code that was still working:

There must have been a day on which it had worked, right?

try this:

git checkout e74463feb2f79ef68a1515e7ad626d8695daa51d
cd subkey

cargo run -- sign-transaction     --call 0300ff408aa36b945720031fbb2e661fc9c9f1ed799be5369f991bbfce02103b5f0e44e514     --nonce 0     --suri //Alice///     --password password     --prior-block-hash f8349fe94e9fd86e8a85745bbb70a4d2d5f4d8fc32a1f8fc08088faf1840e705

that results in:

0x250281ffd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d6c027b50c980a7e8378ce113620db9d26b87db2f8fa1447d9399310c5ea1507c567d2b0d2a8cf03fe114cb42759b242d6af4d6c39ccbae81461b752f7847010500000300ff408aa36b945720031fbb2e661fc9c9f1ed799be5369f991bbfce02103b5f0e44e514

now the question is whether that blob is still compatible with node-template / substrate-2.0.0-3e651110a-x86_64-linux-gnu.

And why the exact same parameters result in a bug in subkey-v2.0.0 sign-transaction ....


P.S.: It's not that that very old version does not have the bug at all, but it just doesn't have the bug for this set of inputs.

drandreaskrueger commented 4 years ago

Now notified these chats: polkadot watercooler (as subkey is not only for substrate, right?) and substrate technical and in polkadot

boneyard93501 commented 4 years ago

first, your --call value needs to be an encoded Call representation, see https://substrate.dev/rustdocs/master/node_runtime/enum.Call.html. If that's not the case, this section in subkey/src/main.rs, ll 477:

let function: Call = hex::decode(&call)
                .ok()
                .and_then(|x| Decode::decode(&mut &x[..]).ok())
                .unwrap();

fails.

however, looking at your use case, it looks like you want to do a balance transfer and can use subkey transfer for that:

subkey transfer  0xd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d  0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48  100000   5 --genesis dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b

from //Alice: 0xd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d to //Bob: 0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 -- genesis: default which gives you something like this:

Using a genesis hash of dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b
0x350284ff78b6dd81f9f55c08fdedb28e5e78e44a1ce6568164d4bd43fa4630a7a38859270184470d96319b269d5f3fc802c6b9a1cda029a52c6b94a170c0c8a65faf0a8f1b2255185f9e0609b608cea812169f1607d211acfe4ac4d86fbe1c50039def418f0014000600ff8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48821a0600

if you to trace the parametrized Call function, you'll get:

Call::Balances(transfer(Address::Id(8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 (5FHneW46...)), 100000))
drandreaskrueger commented 4 years ago

Thanks @boneyard93501 for locating the bug in the code, that will make it easier for you or them to fix it.

In Python we have the concept of "catching an exception" so that end users do not end up with error messages like thread main panicked at called Option::unwrap() on a None value without (for example) explaining WHICH exact user input value was illegally a None value. You have done that now for me, as a human - thanks a lot.

I guess this chapter is the equivalent in rust? An ideal error message should IMHO help me to find out what exactly to fix, or change, so that by varying my input, the command throws no exception anymore.


IF (and not only if) it is decided to let subkey simply crash in such cases, then I am suggesting a subkey sign-transaction ... manual chapter that fully explains the whole thing.


you want to do a balance transfer

Not only, no.
I want to write code that can compose and sign and send an arbitrary extrinsics transaction. But thanks. If I ever want to just transfer a balance, that will help. Cool that subkey can to that.


your --call value needs to be an encoded Call representation,

Yes. I got one of the --call values that I crashes subkey ... via this library's compose_call() function: https://github.com/polkascan/py-substrate-interface/issues/9 BUT even if polkascan's library were still creating a wrong "call" blob, that is NOT the problem (or only a second problem) here, BECAUSE:


long story short:

--call 0300ff408aa36b945720031fbb2e661fc9c9f1ed799be5369f991bbfce02103b5f0e44e514

(Not only MY --code input parameters but) THE ORIGINAL PARAMETER SET FROM MAY 2019 is now throwing the -exact same- error with a 2020 subkey binary:

subkey --version
subkey sign-transaction \
     --call 0300ff408aa36b945720031fbb2e661fc9c9f1ed799be5369f991bbfce02103b5f0e44e514 \
     --nonce 0 \
     --suri //Alice/// \
     --password password \
     --prior-block-hash f8349fe94e9fd86e8a85745bbb70a4d2d5f4d8fc32a1f8fc08088faf1840e705

subkey 2.0.0-alpha.3
Using a genesis hash of dcd1346701ca8396496e52aa2785b1748deb6db09551b72159dcb3e08991025b
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

Thanks a lot.

drandreaskrueger commented 4 years ago

Update about the Python side of things:

(1) the payload is very likely correct

are very very confident that the output of composeCall is correct.

(2) transaction signing MUST happen outside of their library:

signing the composed calls remains off-scope for this project and we will not (cannot) dedicate any resources on this just now. ... may happen in the next few months.

See https://github.com/polkascan/py-substrate-interface/issues/9#issuecomment-597251575

The "may happen in the next few months" is too late for me. And paritytech will probably want to also support other programming languages - e.g. Python - anyways, right?


So please consider to (a) fix the above bug (b) write a manual chapter for Pythonians, OR (c) suggest a 2nd way how in Python (or ANY other language but rust+JS) ... to sign transactions that were generated as such compose_call() results.

Thanks.

drandreaskrueger commented 4 years ago

Surprising outcome:

"Today we learned from Parity Technologies that subkey sign-transaction has not been maintain over a year and we were recommended not to use this option of subkey" (source)


So, no fast rust.

But JS comes to the rescue - a dockerized PolkadotJS wrapper, (performs ~0.6 TPS, but)
IT WORKS. Hooray, and thanks a lot!

See https://riot.im/app/#/room/#polkadot-watercooler:matrix.org/$1583872648119884vTGxz:matrix.parity.io

Great! We have a workaround now, to sign transactions generated in Python, yeehah.


P.S.: Added later. Done multithreading experiments (on an Intel-i5-7th-generation-CPU)
--> I could achieve approx 1.22 TPS with 4 or 6 or 8 multithreading workers, and then after a few more experiments, with 50 threads for 50 signatures I got to 1.27 TPS, and then finally, with 20 threads to solve 20 signatures concurrently, the best value was ...

--> 1.327 TPS

(no that is not 1327 TPS - but about 80 transactions in 60 seconds)

Only for signing. Not composing the extrinsic, mining it into the chain, etc.

So ... I herewith recommend to repair subkey sign-transaction instead. Rust fast, no?

drandreaskrueger commented 4 years ago

Which is the official RPC endpoints documentation (that is valid for the current stable version of substrate / node-template), incl. the scale codec instructions for the parameters of that RPC endpoint? Anyone? Thanks. --> https://github.com/polkascan/py-substrate-interface/issues/10#issuecomment-597737201

drandreaskrueger commented 4 years ago

workaround, ctd.:

Very useful information:

Thanks a lot to Joe!