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

contracts: Convert to framev2 macros #8157

Merged
6 commits merged into from
Feb 22, 2021
Merged

contracts: Convert to framev2 macros #8157

6 commits merged into from
Feb 22, 2021

Conversation

athei
Copy link
Member

@athei athei commented Feb 18, 2021

Convert to the new frame macros.

Metadata diff:

diff --git a/old.json b/new.json
index 2201706..0467169 100644
--- a/old.json
+++ b/new.json
@@ -744,7 +744,7 @@
         "name": "DeletionQueue",
         "modifier": "Default",
         "ty": {
-          "Plain": "Vec<storage::DeletedContract>"
+          "Plain": "Vec<DeletedContract>"
         },
         "default": [
           0
@@ -786,7 +786,7 @@
         },
         {
           "name": "gas_limit",
-          "ty": "Compact<Gas>"
+          "ty": "Compact<Weight>"
         },
         {
           "name": "data",
@@ -812,7 +812,7 @@
         },
         {
           "name": "gas_limit",
-          "ty": "Compact<Gas>"
+          "ty": "Compact<Weight>"
         },
         {
           "name": "code",
@@ -860,7 +860,7 @@
         },
         {
           "name": "gas_limit",
-          "ty": "Compact<Gas>"
+          "ty": "Compact<Weight>"
         },
         {
           "name": "code_hash",
@@ -1206,8 +1206,7 @@
         0
       ],
       "documentation": [
-        " The maximum nesting level of a call/instantiate stack. A reasonable default",
-        " value is 100."
+        " The maximum nesting level of a call/instantiate stack."
       ]
     },
     {
@@ -1220,7 +1219,7 @@
         0
       ],
       "documentation": [
-        " The maximum size of a storage value in bytes. A reasonable default is 16 KiB."
+        " The maximum size of a storage value and event payload in bytes."
       ]
     },
     {
@@ -1252,6 +1251,21 @@
       "documentation": [
         " The maximum amount of weight that can be consumed per block for lazy trie removal."
       ]
+    },
+    {
+      "name": "MaxCodeSize",
+      "ty": "u32",
+      "value": [
+        0,
+        0,
+        2,
+        0
+      ],
+      "documentation": [
+        " The maximum length of a contract code in bytes. This limit applies to the instrumented",
+        " version of the code. Therefore `instantiate_with_code` can fail even when supplying",
+        " a wasm binary below this maximum size."
+      ]
     }
   ],
   "errors": [
@@ -1423,7 +1437,7 @@
       "documentation": [
         " Removal of a contract failed because the deletion queue is full.",
         "",
-        " This can happen when either calling [`Module::claim_surcharge`] or `seal_terminate`.",
+        " This can happen when either calling [`Pallet::claim_surcharge`] or `seal_terminate`.",
         " The queue is filled by deleting contracts and emptied by a fixed amount each block.",
         " Trying again during another block is the only way to resolve this issue."
       ]
@@ -1433,7 +1447,7 @@
       "documentation": [
         " A contract could not be evicted because it has enough balance to pay rent.",
         "",
-        " This can be returned from [`Module::claim_surcharge`] because the target",
+        " This can be returned from [`Pallet::claim_surcharge`] because the target",
         " contract has enough balance to pay for its rent."
       ]
     },

Reduce API surface

Remove some types from the public API that does not need to be exposed. This gives us more leeway to make changes without bumping the major version once v3 is out.

This also seals the Ext trait which is not meant to be implemented by downstream crates. Implementations of it are merely passed to the chain extension for consumption.

@athei athei added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Feb 18, 2021
@athei athei changed the base branch from master to at-code-size-weights February 18, 2021 19:45
@athei athei force-pushed the at-contracts-framev2 branch from 641323c to 913838e Compare February 19, 2021 09:51
Base automatically changed from at-code-size-weights to master February 22, 2021 08:52
@athei athei force-pushed the at-contracts-framev2 branch from 913838e to 36b8a96 Compare February 22, 2021 09:17
@athei athei marked this pull request as ready for review February 22, 2021 09:18
@athei athei added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Feb 22, 2021
@athei athei requested review from ascjones and gui1117 February 22, 2021 09:18
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Show resolved Hide resolved
@gui1117
Copy link
Contributor

gui1117 commented Feb 22, 2021

usually I like to warn about the breaking change: pallet prefix (i.e. the prefix used by storages) is now the name of the pallet given to construct_runtime instead of "Contracts":
Thus we can a runtime-migration flag IMHO, and put this comment in the top comment

#8151 (comment)

frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@athei
Copy link
Member Author

athei commented Feb 22, 2021

usually I like to warn about the breaking change: pallet prefix (i.e. the prefix used by storages) is now the name of the pallet given to construct_runtime instead of "Contracts":
Thus we can a runtime-migration flag IMHO, and put this comment in the top comment

#8151 (comment)

The thing is that almost all changes leading up from v2 to v3 are breaking and would need a storage migration. The thing is that we just do not care what happens to compatibility pre v3. Doing that for this single PR would be misleading?

@gui1117
Copy link
Contributor

gui1117 commented Feb 22, 2021

The thing is that almost all changes leading up from v2 to v3 are breaking and would need a storage migration. The thing is that we just do not care what happens to compatibility pre v3. Doing that for this single PR would be misleading?

yes ok that makes sense, not to do for this single PR

@athei
Copy link
Member Author

athei commented Feb 22, 2021

bot merge

@ghost
Copy link

ghost commented Feb 22, 2021

Waiting for commit status.

@ghost ghost merged commit 4f9ee57 into master Feb 22, 2021
@ghost ghost deleted the at-contracts-framev2 branch February 22, 2021 15:18
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants