`sr25519` signature verification #1757

kziemianek · 2023-04-19T15:51:06Z

Add ability to call contract's sr25519_verify function.

sr25519_verify is still unstable so it's required to run node with type UnsafeUnstableInterface = ConstBool<true>; in pallet-contracts config.

This is related to paritytech/substrate#13703.

crates/env/src/api.rs

crates/env/src/backend.rs

kziemianek · 2023-04-20T07:31:18Z

crates/env/src/engine/off_chain/impls.rs

+        message: &[u8],
+        pub_key: &[u8; 32],
+    ) -> Result<()> {
+        let context = signing_context(b"substrate");


For further use, i think there should be an option to verify signature from any context. Right now there is such limitation.

Could you create a SIGNING_CTX constant like in the substrate and add a comment with the link to the code on the substrate side(that substrate uses the same context)?

goastler · 2023-05-04T08:29:17Z

Hey, was just testing out your changes here which solve my sr25519 signing issue in the ink smart contracts - many thanks for your changes. Just a quick fyi that if the public key is invalid the error is not caught and causes a panic (in the smart contract call, not sure where the panic originates).

I found this from one of my unit tests (https://github.com/prosopo/protocol/blob/81c90b2edde0b71072b1f56b5b3af65a5589e9dc/contracts/lib.rs#L2292) which deliberately uses an incorrect public key. Produces the following error:


---- prosopo::tests::test_verify_sr25519_invalid_public_key stdout ----
data_hash: [70, 251, 116, 8, 212, 242, 133, 34, 143, 74, 245, 22, 234, 37, 133, 27]
data_hex: "46fb7408d4f285228f4af516ea25851b"
payload: <Bytes>0x46fb7408d4f285228f4af516ea25851b</Bytes>
payload_hex: 3c42797465733e307834366662373430386434663238353232386634616635313665613235383531623c2f42797465733e
signature: 0a7da2b631704cdcfe93c740e41217b9ac667a0c8755d8da1a8232db527f487c87e780d2edc1896aeb6b1bef0bc7c38d9df2135b633eab8bfb1777e82fad3a8f
caller AccountId([213, 53, 147, 199, 21, 253, 211, 28, 97, 20, 26, 189, 4, 169, 159, 214, 130, 44, 133, 88, 133, 76, 205, 227, 154, 86, 132, 231, 165, 109, 162, 125])
sig [10, 125, 162, 182, 49, 112, 76, 220, 254, 147, 199, 64, 228, 18, 23, 185, 172, 102, 122, 12, 135, 85, 216, 218, 26, 130, 50, 219, 82, 127, 72, 124, 135, 231, 128, 210, 237, 193, 137, 106, 235, 107, 27, 239, 11, 199, 195, 141, 157, 242, 19, 91, 99, 62, 171, 139, 251, 23, 119, 232, 47, 173, 58, 143]
payload [60, 66, 121, 116, 101, 115, 62, 48, 120, 52, 54, 102, 98, 55, 52, 48, 56, 100, 52, 102, 50, 56, 53, 50, 50, 56, 102, 52, 97, 102, 53, 49, 54, 101, 97, 50, 53, 56, 53, 49, 98, 60, 47, 66, 121, 116, 101, 115, 62]
thread 'prosopo::tests::test_verify_sr25519_invalid_public_key' panicked at 'called `Result::unwrap()` on an `Err` value: PointDecompressionError', /home/geopro/.cargo/git/checkouts/ink-b050680cd543eb5a/9ca19c4/crates/env/src/engine/off_chain/impls.rs:350:68

Happily, incorrect signatures / payload data result in verify returning false. The valid verification test also passes.

goastler · 2023-05-17T16:49:55Z

any update on this? @cmichi @kziemianek :)

kziemianek · 2023-05-24T17:46:47Z

any update on this? @cmichi @kziemianek :)

I'm a bit busy recently. I doubt I'll have a time to look at it in the near future.

forgetso · 2023-05-31T14:20:48Z

@cmichi Would it be possible to merge this given that the review suggestions have been completed? Obviously, after master is merged in. Thx

SkymanOne · 2023-06-03T21:06:09Z

Can you please address the unhandled error resulting in the call panic which was addressed by @goastler ?

SkymanOne · 2023-07-02T21:16:12Z

Hey @kziemianek did you have a change at the review comments?

kziemianek · 2023-07-06T09:59:27Z

Hey @kziemianek did you have a change at the review comments?

Yes, they all are resolved.

goastler · 2023-07-06T18:53:54Z

I have fixed the unhandled errors I was having with this PR, added tests, added docs and fixed the merge conflicts in my PR (#1840) that builds upon this one

xgreenx · 2023-07-16T10:21:38Z

crates/env/src/engine/off_chain/impls.rs

+        message: &[u8],
+        pub_key: &[u8; 32],
+    ) -> Result<()> {
+        let context = signing_context(b"substrate");


Could you create a SIGNING_CTX constant like in the substrate and add a comment with the link to the code on the substrate side(that substrate uses the same context)?

xgreenx · 2023-07-16T10:28:47Z

crates/e2e/macro/src/config.rs

@@ -63,7 +63,7 @@ impl TryFrom<ast::AttributeArgs> for E2EConfig {
                    return Err(format_err_spanned!(
                        arg,
                        "expected a string literal for `additional_contracts` ink! E2E test configuration argument",
-                    ))
+                    ));


Could you exclude this file from the PR, please?=)

xgreenx · 2023-07-16T10:30:10Z

crates/ink/ir/src/ir/item_mod.rs

@@ -263,7 +263,7 @@ impl ItemMod {
                        .into_combine(format_err!(
                            overlap.span(),
                            "first ink! message with overlapping wildcard selector here",
-                        )))
+                        )));


Could you exclude this file from the PR, please?=)

xgreenx · 2023-07-16T10:30:15Z

crates/ink/ir/src/ir/trait_def/item/mod.rs

@@ -384,7 +384,7 @@ impl InkItemTrait {
                ).into_combine(format_err_spanned!(
                    duplicate_selector,
                    "first ink! trait constructor or message with same selector found here",
-                )))
+                )));


Could you exclude this file from the PR, please?=)

xgreenx · 2023-07-16T10:30:43Z

crates/ink/src/env_access.rs

@@ -880,6 +880,16 @@ where
            .map_err(|_| Error::EcdsaRecoveryFailed)
    }

+    pub fn sr25519_verify(


Could you add a comment here as well, please?=)

xgreenx · 2023-07-16T10:44:21Z

Closed in favor of #1840

kziemianek added 4 commits April 19, 2023 14:22

sr25519 signature verification

57c27e3

offchain signature verification

5d9cbee

fmt

37fa479

schnorrkel optional

0a910e1

kziemianek requested review from cmichi, ascjones, HCastano and SkymanOne as code owners April 19, 2023 15:51

kziemianek added 4 commits April 19, 2023 23:35

update dictionary

b2224d6

fix

d6a9f7c

update changelog

9d00c92

add info about unstable function usage

da71ca2

cmichi reviewed Apr 20, 2023

View reviewed changes

crates/env/src/api.rs Outdated Show resolved Hide resolved

cmichi reviewed Apr 20, 2023

View reviewed changes

crates/env/src/backend.rs Show resolved Hide resolved

kziemianek added 2 commits April 20, 2023 08:42

missing docs

eb93905

review suggestions

9ca19c4

kziemianek requested a review from cmichi April 20, 2023 06:54

kziemianek commented Apr 20, 2023

View reviewed changes

goastler mentioned this pull request May 4, 2023

Upgrade ink to version 4.2 prosopo/protocol#273

Open

goastler mentioned this pull request May 31, 2023

use [u8; 64] for signature prosopo/protocol#382

Open

xgreenx reviewed Jul 16, 2023

View reviewed changes

xgreenx closed this Jul 16, 2023

kziemianek deleted the kz-sr25519-verification branch July 16, 2023 12:47

GuiGou12358 mentioned this pull request Sep 15, 2023

Rollup anchor for ink! smart contracts Phala-Network/phat-offchain-rollup#30

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`sr25519` signature verification #1757

`sr25519` signature verification #1757

kziemianek commented Apr 19, 2023

kziemianek Apr 20, 2023

xgreenx Jul 16, 2023

goastler commented May 4, 2023

goastler commented May 17, 2023

kziemianek commented May 24, 2023 •

edited

Loading

forgetso commented May 31, 2023

SkymanOne commented Jun 3, 2023

SkymanOne commented Jul 2, 2023

kziemianek commented Jul 6, 2023 •

edited

Loading

goastler commented Jul 6, 2023

xgreenx Jul 16, 2023

xgreenx Jul 16, 2023

xgreenx Jul 16, 2023

xgreenx Jul 16, 2023

xgreenx Jul 16, 2023

xgreenx commented Jul 16, 2023

sr25519 signature verification #1757

sr25519 signature verification #1757

Conversation

kziemianek commented Apr 19, 2023

kziemianek Apr 20, 2023

Choose a reason for hiding this comment

xgreenx Jul 16, 2023

Choose a reason for hiding this comment

goastler commented May 4, 2023

goastler commented May 17, 2023

kziemianek commented May 24, 2023 • edited Loading

forgetso commented May 31, 2023

SkymanOne commented Jun 3, 2023

SkymanOne commented Jul 2, 2023

kziemianek commented Jul 6, 2023 • edited Loading

goastler commented Jul 6, 2023

xgreenx Jul 16, 2023

Choose a reason for hiding this comment

xgreenx Jul 16, 2023

Choose a reason for hiding this comment

xgreenx Jul 16, 2023

Choose a reason for hiding this comment

xgreenx Jul 16, 2023

Choose a reason for hiding this comment

xgreenx Jul 16, 2023

Choose a reason for hiding this comment

xgreenx commented Jul 16, 2023

`sr25519` signature verification #1757

`sr25519` signature verification #1757

kziemianek commented May 24, 2023 •

edited

Loading

kziemianek commented Jul 6, 2023 •

edited

Loading