Repository: paritytech/ink
Pull request #1123 (2022-02-07 15:00:00)

Enable use of global secp256k1 context
Context creation is expensive.

Use the preallocated context that ships with the library.

Follows from the work done for this https://github.com/paritytech/substrate/pull/10798

paritytech-cicd-pr: ## 🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑
 These are the results when building the `examples/*` contracts from this branch with `cargo-contract 0.17.0-b2a35cd` and comparing them to ink! `master`: 

|                      | Δ Optimized Size | Δ Used Gas | Total Optimized Size | Total Used Gas |
|:----------------------|------------------:|------------:|----------------------:|----------------:|
| `accumulator`        |                  |            | 1.44 K               |                |
| `adder`              |                  |            | 2.94 K               |                |
| `contract-terminate` |                  |            | 1.31 K               | 200_778        |
| `contract-transfer`  |                  |            | 7.98 K               | 1_399          |
| `delegator`          |                  |            | 7.63 K               | 10_296         |
| `dns`                |                  |            | 10.20 K              | 15_153         |
| `erc1155`            |                  |            | 27.50 K              | 47_811         |
| `erc20`              |                  |            | 9.80 K               | 10_014         |
| `erc721`             |                  |            | 13.70 K              | 34_702         |
| `flipper`            |                  |            | 1.76 K               | 347            |
| `incrementer`        |                  |            | 1.64 K               | 340            |
| `multisig`           |                  |            | 26.79 K              | 56_756         |
| `proxy`              |                  |            | 3.81 K               | 2_629          |
| `rand-extension`     |                  |            | 5.12 K               | 6_654          |
| `subber`             |                  |            | 2.96 K               |                |
| `trait-erc20`        |                  |            | 10.10 K              | 10_659         |
| `trait-flipper`      |                  |            | 1.53 K               | 342            |
| `trait-incrementer`  |                  |            | 1.62 K               | 680            |



[Link to the run](https://gitlab.parity.io/parity/ink-waterfall/-/pipelines/178130) | Last update: Mon Feb  7 16:53:51 CET 2022
codecov-commenter: # [Codecov](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech) Report
> Merging [#1123](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech) (c57783d) into [master](https://codecov.io/gh/paritytech/ink/commit/b9202db401153ab0bae0d6ef3b488ee09bc69142?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech) (b9202db) will **increase** coverage by `15.11%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/paritytech/ink/pull/1123/graphs/tree.svg?width=650&height=150&src=pr&token=tYyVaCZb7o&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech)](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech)

```diff
@@             Coverage Diff             @@
##           master    #1123       +/-   ##
===========================================
+ Coverage   63.70%   78.81%   +15.11%     
===========================================
  Files         252      252               
  Lines        9397     9392        -5     
===========================================
+ Hits         5986     7402     +1416     
+ Misses       3411     1990     -1421     
```


| [Impacted Files](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech) | Coverage Δ | |
|---|---|---|
| [crates/engine/src/tests.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2VuZ2luZS9zcmMvdGVzdHMucnM=) | `100.00% <ø> (ø)` | |
| [crates/engine/src/ext.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2VuZ2luZS9zcmMvZXh0LnJz) | `72.22% <100.00%> (+6.70%)` | :arrow_up: |
| [...tes/env/src/engine/experimental\_off\_chain/impls.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2Vudi9zcmMvZW5naW5lL2V4cGVyaW1lbnRhbF9vZmZfY2hhaW4vaW1wbHMucnM=) | `53.70% <100.00%> (-0.43%)` | :arrow_down: |
| [crates/env/src/engine/off\_chain/impls.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2Vudi9zcmMvZW5naW5lL29mZl9jaGFpbi9pbXBscy5ycw==) | `41.88% <100.00%> (-0.50%)` | :arrow_down: |
| [crates/lang/ir/src/ir/attrs.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2xhbmcvaXIvc3JjL2lyL2F0dHJzLnJz) | `82.27% <0.00%> (+3.60%)` | :arrow_up: |
| [crates/allocator/src/bump.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2FsbG9jYXRvci9zcmMvYnVtcC5ycw==) | `66.66% <0.00%> (+4.16%)` | :arrow_up: |
| [crates/lang/ir/src/ir/trait\_def/item/mod.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2xhbmcvaXIvc3JjL2lyL3RyYWl0X2RlZi9pdGVtL21vZC5ycw==) | `90.16% <0.00%> (+4.91%)` | :arrow_up: |
| [crates/lang/ir/src/ir/item\_impl/constructor.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2xhbmcvaXIvc3JjL2lyL2l0ZW1faW1wbC9jb25zdHJ1Y3Rvci5ycw==) | `93.84% <0.00%> (+6.15%)` | :arrow_up: |
| [crates/lang/ir/src/ir/idents\_lint.rs](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech#diff-Y3JhdGVzL2xhbmcvaXIvc3JjL2lyL2lkZW50c19saW50LnJz) | `71.42% <0.00%> (+9.52%)` | :arrow_up: |
| ... and [43 more](https://codecov.io/gh/paritytech/ink/pull/1123/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech). Last update [b9202db...c57783d](https://codecov.io/gh/paritytech/ink/pull/1123?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paritytech).

xgreenx: ink! doesn't use `secp256k1` to recover, generate keys. So it doesn't use the context. Usage of the context will increase the size of the contracts. It is why that function was moved to `contract-pallet`.

Maybe I'm not right and it doesn't affect the size of the contract, could you check it, please?=)
davxy: > ink! doesn't use `secp256k1` to recover, generate keys. So it doesn't use the context. Usage of the context will increase the size of the contracts. It is why that function was moved to `contract-pallet`.
> 
> Maybe I'm not right and it doesn't affect the size of the contract, could you check it, please?=)

@xgreenx Ink! has already replaced `libsecp256k1` with `secp256k1` here https://github.com/paritytech/ink/pull/1052

If you check the changed files, this PR only introduces the usage of the global `secp256k1` feature 
xgreenx: Ohh, okay, you only enabled that feature for tests. You didn't touch [the code related to contracts](https://github.com/paritytech/ink/blob/master/crates/eth_compatibility/Cargo.toml#L24).

Patch:
diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml
index 3f5851e827..63428716e6 100644
--- a/crates/engine/Cargo.toml
+++ b/crates/engine/Cargo.toml
@@ -23,10 +23,11 @@ sha3 = { version = "0.10" }
 blake2 = { version = "0.10" }
 
 # ECDSA for the off-chain environment.
-secp256k1 = { version = "0.21.2", features = ["recovery"] }
+secp256k1 = { version = "0.21.2", features = ["recovery", "global-context"], optional = true }
 
 [features]
 default = ["std"]
 std = [
     "scale/std",
+    "secp256k1"
 ]
diff --git a/crates/engine/src/ext.rs b/crates/engine/src/ext.rs
index 7cef11a46e..0eb2d7e5cd 100644
--- a/crates/engine/src/ext.rs
+++ b/crates/engine/src/ext.rs
@@ -391,7 +391,7 @@ impl Engine {
                 RecoveryId,
             },
             Message,
-            Secp256k1,
+            SECP256K1,
         };
 
         // In most implementations, the v is just 0 or 1 internally, but 27 was added
@@ -414,8 +414,7 @@ impl Engine {
                     panic!("Unable to parse the signature: {}", error)
                 });
 
-        let secp = Secp256k1::new();
-        let pub_key = secp.recover_ecdsa(&message, &signature);
+        let pub_key = SECP256K1.recover_ecdsa(&message, &signature);
         match pub_key {
             Ok(pub_key) => {
                 *output = pub_key.serialize();
diff --git a/crates/env/Cargo.toml b/crates/env/Cargo.toml
index 92e68498a5..3b27e5db79 100644
--- a/crates/env/Cargo.toml
+++ b/crates/env/Cargo.toml
@@ -32,7 +32,7 @@ static_assertions = "1.1"
 rlibc = "1"
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
-ink_engine = { version = "3.0.0-rc8", path = "../engine/", default-features = false, optional = true }
+ink_engine = { version = "3.0.0-rc8", path = "../engine/", optional = true }
 
 # Hashes for the off-chain environment.
 sha2 = { version = "0.10", optional = true }
@@ -40,7 +40,7 @@ sha3 = { version = "0.10", optional = true }
 blake2 = { version = "0.10", optional = true }
 
 # ECDSA for the off-chain environment.
-secp256k1 = { version = "0.21.2", features = ["recovery"] }
+secp256k1 = { version = "0.21.2", features = ["recovery", "global-context"], optional = true }
 
 # Only used in the off-chain environment.
 #
@@ -60,6 +60,7 @@ std = [
     "scale/std",
     "scale-info",
     "scale-info/std",
+    "secp256k1",
     "rand",
     "rand/std",
     "rand/std_rng",
diff --git a/crates/env/src/engine/experimental_off_chain/impls.rs b/crates/env/src/engine/experimental_off_chain/impls.rs
index 87501d3e27..6293254864 100644
--- a/crates/env/src/engine/experimental_off_chain/impls.rs
+++ b/crates/env/src/engine/experimental_off_chain/impls.rs
@@ -257,7 +257,7 @@ impl EnvBackend for EnvInstance {
                 RecoveryId,
             },
             Message,
-            Secp256k1,
+            SECP256K1,
         };
 
         // In most implementations, the v is just 0 or 1 internally, but 27 was added
@@ -278,8 +278,7 @@ impl EnvBackend for EnvInstance {
                     panic!("Unable to parse the signature: {}", error)
                 });
 
-        let secp = Secp256k1::new();
-        let pub_key = secp.recover_ecdsa(&message, &signature);
+        let pub_key = SECP256K1.recover_ecdsa(&message, &signature);
         match pub_key {
             Ok(pub_key) => {
                 *output = pub_key.serialize();
diff --git a/crates/env/src/engine/off_chain/impls.rs b/crates/env/src/engine/off_chain/impls.rs
index 98817c2cc3..625ddd2e72 100644
--- a/crates/env/src/engine/off_chain/impls.rs
+++ b/crates/env/src/engine/off_chain/impls.rs
@@ -203,7 +203,7 @@ impl EnvBackend for EnvInstance {
                 RecoveryId,
             },
             Message,
-            Secp256k1,
+            SECP256K1,
         };
 
         // In most implementations, the v is just 0 or 1 internally, but 27 was added
@@ -224,8 +224,7 @@ impl EnvBackend for EnvInstance {
                     panic!("Unable to parse the signature: {}", error)
                 });
 
-        let secp = Secp256k1::new();
-        let pub_key = secp.recover_ecdsa(&message, &signature);
+        let pub_key = SECP256K1.recover_ecdsa(&message, &signature);
         match pub_key {
             Ok(pub_key) => {
                 *output = pub_key.serialize();


Test patch:
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index b26a1c9b2a..0521647384 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -20,8 +20,8 @@ use secp256k1::{
     ecdsa::RecoverableSignature,
     Message,
     PublicKey,
-    Secp256k1,
     SecretKey,
+    SECP256K1,
 };
 
 /// The public methods of the `contracts` pallet write their result into an
@@ -238,7 +238,6 @@ fn ecdsa_recovery_test_from_contracts_pallet() {
 fn ecdsa_recovery_with_secp256k1_crate() {
     // given
     let mut engine = Engine::new();
-    let secp = Secp256k1::new();
     let seckey = [
         59, 148, 11, 85, 134, 130, 61, 253, 2, 174, 59, 70, 27, 180, 51, 107, 94, 203,
         174, 253, 102, 39, 170, 146, 46, 252, 4, 143, 236, 12, 136, 28,
@@ -255,7 +254,7 @@ fn ecdsa_recovery_with_secp256k1_crate() {
     let msg = Message::from_slice(&msg_hash).expect("message creation failed");
     let seckey = SecretKey::from_slice(&seckey).expect("secret key creation failed");
     let recoverable_signature: RecoverableSignature =
-        secp.sign_ecdsa_recoverable(&msg, &seckey);
+        SECP256K1.sign_ecdsa_recoverable(&msg, &seckey);
 
     let recovery_id = recoverable_signature.serialize_compact().0.to_i32() as u8;
     let mut signature = recoverable_signature.serialize_compact().1.to_vec();