WIP 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.
• 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.

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.

[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.