Polkadot v1.1.0

[gianfra-t]

@pendulum-chain/product: This PR adds changes to the node client code that require a redeployment of the collator nodes to take effect. Please ensure that the collator nodes are redeployed after this PR is merged.

Closes #481 .

Notable changes and decisions.

• Remove implementation of the trait Convert<A,B>, since it is not available anymore. (previous definition, cannot be found anymore on the same module) and it is no longer required (no compilation error on runtimes).
• Farming pallet. We thought that on version 1.1.0 they had already introduced our abstraction, this is not the case. It only appears on later releases. Instead of using the 1.1.0 branch, we can use commit 46ba3689c2fe1011cce0d752cb479e0e522e2e76 which is when the feature was merged into develop branch, at that time using 1.1.0.
• Node. Modification on how the aura service params are declared and started. There is no need to call start_collator anymore since it is all started in start_consensus function. Example.
• start_full_node is deprecated, the correct way to start the relay chain functions is now using the following. Note: The behavior is the same, since previously start_collator which is also deprecated, would start anyway the relay chain functions.
• pallet-contracts new types added. Decisions on values to take are Hash Lockup Deposit and max delegate dependency For Environment, only valid value is ().
• Contract's runtime API's use of UnsafeDebug here. I believe it is fine to use them since as per comments on the pallet, should only be used for RPC. Same with UnsafeCollect.
• pallet-xcm's new type MaxRemoteLockConsumers. See changes here. I do not fully understand the implications of this value and I've chosen 0 to maintain functionality. Related PR and changelog.

Integration tests changes.

Now we differentiate between the xcm-simulator and xcm-emulator. See the small wiki here. Previous macros now belong to the simulator, while from the description I infer that what we actually need for integration tests is the emulator. I also tested the simulator since it required less changes but I was getting Unroutable errors for every xcm message sent. Therefore, for these two reasons I decided to move to the emulator.

Most of the code changes revolve around the new macro definitions and most importantly the genesis config of these test chains.

Asset-hub runtimes issue.

Initially it was attempted to get the runtimes from the new fellows repo. The issue is that it uses the crates.io version of the polkadot dependencies, which brought conflicts when using the unit tests, since all our dependencies are from the polkadot-sdk repo, branch release-polkadot-v1.1.0. Although they are the same to release-crates-io-v1.1.0, the version scheme changes. The solution to this issue was to implement a patch with all packages specified to normalize for this branch (attempted in this commit). Previous macros for building the externalities are now replaced by the genesis config.

Alternatively, we can use the runtimes from the polkadot-sdk repository, although they seem to be fixed at version 9.42 of the runtime, while maintaining compatibility with the dependencies. Since this is only used for unit testing I have implemented this second option to avoid the maintaining the patch. The alternative is also possible if we require so in the future.

Testing Migrations

Related ticket. Only pallet_contracts seems to define a migration for this update. In this case, from 10 to 15. Also, both pallet_bounties and parachain_staking need to be bumped to the correct version (4 and 7 respectively). According to this PR, the migration for Bounties must happen because the prefix was changed. Now, while running these migrations as defined on the companion update for Kusama/Polkadot at that time, we get that the new prefix is not empty (we get this error), hinting that the storage already lives on the new prefix.

We can test the application of this migrations with the try-runtime tool. Command to run it and docs in our notion. The output of this should tell us if the migrations will be applied correctly or not.

• [x] Try-runtime output for Foucoco
• [x] Try-runtime output for Amplitude
• [x] Try-runtime output for Pendulum

Bandersnatch_vfrs patch

Addition of this patch exception was required due to an error in compilation with nightly-2024-04-18, see run issue. Using a new nightly version solves the issue (nightly-2024-05-30 for instance), but the package bandersnatch_vrfs was not found. Solution found here.

Open questions

• The parameters passed to closure used to create inherents changed and we no longer have access to validation_data. If we still want to provide a parachain_inherent aside from timestamp, we need to figure out how to create it.

As far as I see from other chains and templates (bifrost, sdk template) it is okay to pass an empty closure since the pallet will take care of adding the inherents by default.

• It is still unclear to me why for some integration tests we have to use a different "native base fee". The multiples are still consistent with what is defined on the asset regsitry, but for example the base fee taken when transferring dot from relay chain is larger than when transferring native. My current hypothesis is that extra operations (or more "costly" ones) are involved when receiving non-native assets.

[prayagd]

redeployment of the collator nodes to take effect.

Do we have a ticket for this, so i can link here as blocked?

[gianfra-t]

@prayagd they have to be redeployed but after the changes on this PR are finished. Sorry it pinged you since it is still a draft.

[gianfra-t]

@ebma about this question:

But these patches are not contained in the most recent version anymore. Does this mean you want to rely on the current state of the Cargo.lock file? Seems a little brittle to me. Or am I missing something here?

We are not relying on the lock file, we required the patches if we were to use the assethub runtimes from the polkadot-fellows repo. But if we use the assethub runtimes from the polkadot-sdk directly, then there is no compilation issue. The difference as I put in the description is that these are fixed at 0.9.42, even though the sdk version is 1.1.0. I believe they just made them compatible but did not upgrade the logic itself. So we have the choice to use either:

• Patch + runtimes from the polkadot-fellows repo.
• Runtimes from the polkadot-sdk

When I discovered the second option was also working, I chose that one since maintaining the patch is very brittle. Especially since we cannot do cargo update without breaking our lock.

About the weights, yes I didn't think about regenerating the weights again. Do you think it can be a bit too much for a single PR?

[ebma]

Thanks for explaining that part again 👍 Yes I fully agree that not having to patch all dependencies is the better approach, so let's stick to the runtimes from the polkadot-sdk repository.

About the weights, yes I didn't think about regenerating the weights again. Do you think it can be a bit too much for a single PR?

I would opt for including it in the same PR as the weights are only required due to the updated dependencies and we shouldn't roll out the runtime upgrade without regenerating them first. I would hope that this is not too much overhead.

[gianfra-t]

I updated the weights by running the benchmarks again. Created a simple script for this that will run all the existing benchmarks on the weights folder, ignoring fails. @pendulum-chain/devs If you like this script we can keep it, I'll add it to the REAMDE.

Also added a fix for the failing orlm-tokens-management which only required to increase the balance of the tester account. I did it this way to avoid conversion errors (since in benchmarks is always hard to access values).

Only pallet redeem is not working for the pendulum runtime.

called `Result::unwrap()` on an `Err` value: Module(ModuleError { index: 53, error: [5, 0, 0, 0], message: Some("ExistentialDeposit") }) 

This is a bit surprising since also amplitude has the same existential deposit values, yet it works there.

We can either choose to investigate further, or use amplitude's weights for this pallet, depending on priorities.

EDIT: Should also add that the benchmark also fails in main.

[ebma]

I briefly compared the runtime definitions of pendulum and amplitude but was not able to spot the difference that could make the benchmark fail on pendulum. @gianfra-t which one of the benchmark functions fails exactly?

I'm happy to keep your shell script for the weight generation, thanks for creating it 👍

[gianfra-t]

When you benchmark on main is harder to see, but running on this branch there is an unwrap() panicking, this line precisely.

So clearly the issue must be closer to the VaultRegistry config. But as you said, I don't understand how if the ED is the same. Do you recall any liquidation parameter that could be different between runtimes?

[ebma]

I'm not 100% sure but maybe it's because of the differing values for the thresholds (e.g. this) of the vault registry in the genesis configuration of pendulum vs amplitude in chain_spec.rs. Can you try if changing all the thresholds to the same values as are used in Amplitude fixes the benchmark for Pendulum?

[gianfra-t]

Nope, same error 🤔.

[gianfra-t]

@ebma I pushed a change to increase the amount of tokens issued before liquidating and this solves the fix. I suspect this line and subsequent transfer, but I have no idea why it worked on Amplitude. In any case I executed both again with the same parameter.

Technically we should point to a new rev once the Spacewalk fix is merged.

@pendulum-chain/devs let me know if I missed an open question or remark since this PR grew a bit too much already!

[ebma]

Although we don't 100% know which caused the issue, the amount used was very low so the fix is fine with me. Let's point to the new Spacewalk rev then.

[gianfra-t]

Sorry @b-yap, but if you can re-approve again please since @ebma and I where last to commit and now the review is stale.

[ebma]

I'll just merge it for you @gianfra-t.