Release 1.15.1 #854
Open
Release 1.15.1 #854
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Code paths changed which broke documentation that replied on this file.
Fixes KILTprotocol/ticket#3325. Add some basic lints (can always be expanded for either runtime-specific code or all code via the configurable rustc flags in the new config file). I updated the code to address all places in which the new lints generate an error, including all weight files and weight templates. I also updated the CI check step to check for the specific WASM-targeted, no-std code that will be part of our runtime. ## How to test To check runtime code against the new lint, just run the same command as in the check workflow file: `cargo clippy --locked --target wasm32-unknown-unknown --no-default-features --workspace --exclude kilt-parachain --exclude standalone-node --exclude xcm-integration-tests --exclude 'dip-provider*' --exclude 'dip-consumer*'`
Add more lints to our codebase. I have taken all lints that are allowed by default, and made either either warnings or strong denies. **I have not added any lints from the [pedantic](https://rust-lang.github.io/rust-clippy/master/index.html?groups=pedantic) group, as those tend to be opinionated and I don't think should be enforced on a shared codebase.** Unfortunately, there is an [open cargo issue](rust-lang/cargo#8170) which makes it impossible to enable/disable lints based on the `#[cfg(test)]` or `#[cfg(feature = "...")]` features, hence I have had to enable lints for either the whole codebase (`[target.'cfg(all())']`), or for runtime-only code `[target.'cfg(target_arch = "wasm32")']`. The changeset is huge but just because I tackled all the new lints generated, so reviewing the new lints (in case some are unwanted or in case I missed some) would be sufficient, since the CI has been updated to verify all lints are properly addressed. Last but not least, I had to enable the `-Dwarnings` flag for ALL clippy invocations, so also on local machines, because it's not possible to pass custom `--cfg` either to rustc via cargo, e.g., `cargo clippy -- --cfg ci` and then have a `[target.'cfg(ci)']` entry in the config file. This might go away in future versions of cargo (we are currently on 1.74), but until then we either never fail on the CI, or we fail for everything also locally, and I think the latter is a better choice. This is also because rustc takes additional flags either from the `RUSTFLAGS` env variable or the `.cargo/config.toml` file. Hence specifying `RUSTFLAGS="-Dwarnings"` in the CI would not enable any additional lints as the env variable would take precedence over the config file, rendering them useless.
Part of KILTprotocol/ticket#3650. Makes the web3name pallet instantiatable. I did not rename the pallet/crate itself, and was not sure if I wanted to do it. That could break more things and I thought we might want to leave everything else as-is.
Part of KILTprotocol/ticket#3650. Built on top of #779.
Part of KILTprotocol/ticket#3650. Built on top of #781. ## Trade-off I chose to go this way instead of providing an optional counter, because providing a counter would require one of the following two approaches: 1. Transform the storage double map into a counted one, requiring a migration also for our currently-deployed pallet, which I wanted to avoid 2. Not use a counter, but iterate every time to make sure there are still "spots" left for the current DID. This would require changing the benchmarking logic as now we have a potentially unbounded iteration happening. I also wanted to avoid that. Hence, the solution was to provide a somehow more limited feature of simply specifying whether the links are expected to be unique per DID or not. This, as long as we set `false` for our deployed pallets would not require any storage migration, and does not require any changes in the benchmarks, so I found it a good compromise.
Part of KILTprotocol/ticket#3650, built on top of #782. I left a few comments on the review to help the reviewer understanding the context of this changeset. ## Checklist - [x] Add dotnames pallet to both runtimes - [x] Add second linking deployment with unique linking to both runtimes - [ ] ~Add new runtime API for batch resolution to both runtimes~ -> will do in a separate PR along with a new runtime API for unique linking resolution
Part of KILTprotocol/ticket#3650, based on top of #784. Last bit of required functionality. ## Features * Since it's not possible to batch multiple runtime API calls, I introduced a `batch_` version for our old `did` runtime API, which allows to do batching instead of firing one request per lookup * Similarly to that, I introduced a new runtime API (since it's not possible to implement the same API twice for the same runtime), which resolves an account given a name, or a name given an account. This new API also provides a batched version to resolve multiple names or addresses at once. This was one of the requirements we got from Parity to make the solution easier to integrate in the mobile app ## How to test Polkadot Apps support new runtime APIs out of the box. So just compile Peregrine or Spiritnet runtime and spin up a network with Chopsticks. Then: 1. Create a new account-controlled DID. For the Sudo key, use this call `0x401003921cbc0ffe09a865dbf4ae1d0410aa17c656881fe86666da0f97939e3701b674` 2. Claim a new dotname. For the Sudo key, use this call `0x400f921cbc0ffe09a865dbf4ae1d0410aa17c656881fe86666da0f97939e3701b674490020746573742e646f74` 3. Link the Sudo account to the DID. Use this call `0x400f921cbc0ffe09a865dbf4ae1d0410aa17c656881fe86666da0f97939e3701b6744a01` 4. Call the runtime API `uniqueLinking.addressForName("test.dot")` which should return ``` { Ok: { address: { AccountId32: 4rDeMGr3Hi4NfxRUp8qVyhvgW3BSUBLneQisGa9ASkhh2sXB } extra: 4rDeMGr3Hi4NfxRUp8qVyhvgW3BSUBLneQisGa9ASkhh2sXB } } ``` 5. Call the runtime API `uniqueLinking.nameForAddress({AccountId32: "4rDeMGr3Hi4NfxRUp8qVyhvgW3BSUBLneQisGa9ASkhh2sXB"})` which should return ``` { Ok: { name: test.dot extra: 4rDeMGr3Hi4NfxRUp8qVyhvgW3BSUBLneQisGa9ASkhh2sXB } } ```
The CI started failing because a Substrate dependency has gone officially unmaintained. We start ignoring that dependency and will re-evaluate at the next polkadot-sdk upgrade what to do.
~So that we can enable the `"-Wclippy::tests_outside_test_module"` flag. Thanks @Ad96el for this.......~ I ignored the lint for pallets using the v1 benchmarking macros, and for our emulated tests.
Part of KILTprotocol/ticket#3650, built on top of #787. What started as a simple fix for the web3name pallet benchmarks, turned out to be a bigger changeset due to a couple of bugs found in the Substrate code, which are mentioned in the self-review I gave myself [below](#790 (review)). In a gist, what I did is: 1. Update the benchmarking logic for the web3name pallet to delegate the name creation to a `BenchmarkHelper` type. 2. Implement the trait above for both web3names (which uses the old implementation) and the dotnames 3. Work around the Substrate bug for our pallets as well as the `pallet_collective` and `pallet_membership` pallets which we deploy multiple times 4. Add the benchmarking of the `pallet_membership` deployments of the `TipsMembership` pallet
WIP. Fixes KILTprotocol/ticket#3672. Built on top of #790. ## Checklist - [x] Review Peregrine code - [x] Ask for review - [x] Apply to Spiritnet code
Fixes KILTprotocol/ticket#3688. Due to how the `impl_runtime_apis` macro works, it is expected to be included in a runtime's `lib.rs` file. Since we split up the runtimes, the runtime APIs were not included anymore. I found a related issue in the subxt repo: paritytech/subxt#1873. Because the trait definition generated by the macro is local, we can't import it in the `lib.rs` module for the `construct_runtime` macro to pick up the right implementation, so we need a workaround that basically introduces a new marker trait, and we import _that_ in the `lib.rs` file so that the right implementation of `Runtime::runtime_metadata()` is picked up. More details about the issue are presented in the subxt ticket. ## How to test Spin up a chopsticks deployment after building peregrine and spiritnet runtime, and verify that the runtime APIs are there now.
I updated the weights with the proper (production) ones for DID, both lookup deployments, both web3name deployments, sudo, and utility pallets. --------- Co-authored-by: ad96el <[email protected]>
Fixes KILTprotocol/ticket#3691. This PR allows to dynamically (i.e., via the sudo origin) elect an account as the only authorised to submit `claim` extrinsic for any web3name pallet deployment, and `associate_account` and `associate_sender` extrinsics for any did-lookup deployment. I also updated our dotnames and unique linking deployments for both our runtimes to use this new feature, althought I had to rely on a trick with `storage_alias`es to avoid introducing a new pallet just for this. Unfortunately, the [pallet-parameters](https://github.com/paritytech/polkadot-sdk/tree/master/substrate/frame/parameters) was only introduced in 1.8.0, so we might want to migrate to using that once we update our codebase to 1.8.0. For now, I could not think of any other way to implement this feature without touching the pallets storage entries, which I did not want to. For both our runtimes, these are the two new storage keys introduced: * `0x8ea135058ec16554c8e3d230d658fbffd30ff375811804de60521a1654f58ebb` for the dotnames deployment authorization * `0x41a63f711fa40ef5e1dc8f0ac115a906d4378bcb7f1d95ba1124c2140bfccdba` for the unique linking deployment authorization These values can be updated with a `system.setStorage(key, value)` call, which must specify an account ID as the sole submitter of the extrinsics specified above. A `system.killStorage(keys)` call will set the relative entry to `None`, which means no gating is enforced and anyone can create. This is the default for our web3name and did linking deployments. **Important: when deploying the new runtime, we will also need to set the storage value for these entries, and we don't need to wait for the new runtime to be live as writing it earlier has no effect on the rest of the runtime**. ## How to test 1. Spin up a chopsticks Peregrine setup 2. Set a desired account as the sole allowed submitter for dotnames * E.g., This call sets it to the sudo `0x000404808ea135058ec16554c8e3d230d658fbffd30ff375811804de60521a1654f58ebb80921cbc0ffe09a865dbf4ae1d0410aa17c656881fe86666da0f97939e3701b674` 4. Try to claim a dotname with any DID while submitting the tx with an account different than the sudo account: this will not work 5. Try again with the sudo account and it will work. 6. Remove the authorised account with `killStorage` * This call removes it: `0x000504808ea135058ec16554c8e3d230d658fbffd30ff375811804de60521a1654f58ebb` 8. Try again with the first account: it will now work.
## fixes KILTprotocol/ticket#3600 Maintain only two versions of docker images on the public registry. `kiltprotocol/kilt-node` `kiltprotocol/standalone-node` ## Metadata Diff to Develop Branch <details> <summary>Peregrine Diff</summary> ``` ``` </details> <details> <summary>Spiritnet Diff</summary> ``` ``` </details> ## Checklist: - [ ] I have verified that the code works - [ ] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [ ] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [ ] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [ ] I have documented the changes (where applicable) * Either PR or Ticket to update [the Docs](https://github.com/KILTprotocol/docs) * Link the PR/Ticket here
## fixes NO TICKET minor fix to make the scripts executable to resolve `/usr/bin/bash: line 167: ./.maintain/build-image.sh: Permission denied` on gitlab CI ## Checklist: - [ ] I have verified that the code works - [ ] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [ ] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [ ] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [ ] I have documented the changes (where applicable) * Either PR or Ticket to update [the Docs](https://github.com/KILTprotocol/docs) * Link the PR/Ticket here
Fixes KILTprotocol/ticket#3702. This PR implements the `MutateHold` trait for the deposit storage pallet by introducing a `PalletDepositStorageReason` type that, in the runtime, is required to implement the runtime's `RuntimeHoldReason`. Because the deposit pallet assumes that the deposit payer can reclaim the deposit back, we can't add the `MutateHold` implementation without either 1. requiring a migration to distinguish which deposits are reclaimable and which are not, or 2. introducing a new storage map that only keeps track of "system" deposits, aka deposits that are part of some runtime logic and that are only supposed to be changed by interacting with the `MutateHold` trait and not directly via the pallet exposed extrinsic(s). ## How to use it in the bonding curves When switching to the deposit pallet as the deposit fungible, the bonding curve pallet has to define a namespace, and a unique key per deposit to be taken. For instance, the tuple (asset ID, account ID) could be such a unique key. Example of integration PR: #827. I can provide support for integrating this into the runtime once the runtimes have been updated to have the required pallets.
## fixes KILTprotocol/ticket#3585 Co-authored-by: Raphael <[email protected]> Co-authored-by: Raphael Flechtner <[email protected]> Co-authored-by: Antonio <[email protected]>
...Opps
We introduce a new feature for peregrine and spiritnet runtimes, which adds the metadata hash signed extension, for wallets that need it. The Gitlab build pipeline has also been updated to include the new feature only when building the production WASMs. ## How to test Change any integration tests in the SDK based on this PR (KILTprotocol/sdk-js#924), and verify the metadata is verified (e.g., by enabling logs via a Chopsticks deployment).
The node is used in SDK integration tests, so we need to have it in there to make sure having support for it works fine.
## fixes NO TICKET The `Organisation` was missing from the docker tag ## Checklist: - [ ] I have verified that the code works - [ ] No panics! (checked arithmetic ops, no indexing `array[3]` use `get(3)`, ...) - [ ] I have verified that the code is easy to understand - [ ] If not, I have left a well-balanced amount of inline comments - [ ] I have [left the code in a better state](https://deviq.com/principles/boy-scout-rule) - [ ] I have documented the changes (where applicable) * Either PR or Ticket to update [the Docs](https://github.com/KILTprotocol/docs) * Link the PR/Ticket here
Right now, if the new runtime is enacted and there's no authorised account set, the calls are considered permissionless. To avoid this, and to allow us to set an account later on, I changed the logic so that by default it's considered unusable, and only when an account is set, the stored account is checked. I introduced also a new type to allow for permissionless dispatch. The change does not require any migrations or changes to storage, as it simply introduces a semantic change to how those storage values are interpreted. ## How to test Use chopsticks to try to claim a DOT name before anything is set, it will fail with `BadOrigin`. Set the new value with a `sudo.setStorage` call, and verify that only that account can submit `dotname.claim` or `uniqueLinking.linkAccount` transactions.
# Fixes: [#3746](KILTprotocol/ticket#3746)
Required for the `CheckMetadata` extension to work properly. Fixes KILTprotocol/ticket#3744. Changes "inspired" from this PR: paritytech/cumulus#1054. ## How to test We can re-test this and other related scenarios during our deployment to staging. I already included the step in the [next release ticket](KILTprotocol/ticket#3524). ## Checklist - [x] Check block production for standalone (binary) - [x] Check if SDK metadata-related tests pass - [x] Check that the other commands (e.g., benchmarks) work fine - [x] Check block production for kilt-parachain (locally built Docker image + Zombienet)
## fixes [#3747](KILTprotocol/ticket#3747)
# Fixes: [#3745](KILTprotocol/ticket#3745)
Fixes KILTprotocol/ticket#3673. I defined a new collection of hooks for the DID pallet, which can be extended with new functions if we need to check conditions for some other DID operations. I provide a bunch of implementations to check for related resources. ~The only limitation to the current solution is that we don't have a precise worst case weight calculation for the hook, since we would need to benchmark the storage accesses, and the pallets use the old benchmarking logic which is not very flexible. I will work on the benchmarks and on updating the actual weights in a separate PR.~ Apparently, [it's enough to use the expected `MaxEncodedLen` as the proof size](https://substrate.stackexchange.com/questions/11842/calculate-proof-size-of-a-function-that-reads-state/11843#11843), so I switched to that in the [final commit](72fc198). ## How to test Spin up a Chopsticks instance, claim a DID, claim a web3name, try to delete the DID: it will fail. Delete the web3name. Try to delete the DID: it will succeed. The same can be tried with linked accounts.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.