pendulum-chain / pendulum

GNU General Public License v3.0
43 stars 14 forks source link

Update to 0.9.42 #382

Closed gianfra-t closed 6 months ago

gianfra-t commented 9 months ago

Note: I will leave this as draft until merged with the fix code for try-runtime.

@pendulum-chain/product: This PR adds changes to the node client code that require a redeployment of the collator nodes to take effect.

Closes tasks/#158 Closes #222. Closes https://github.com/pendulum-chain/tasks/issues/64.

Updates to dependencies

Spacewalk -> branch upgrade-v0.9.42 Substrate-stellar-sdk -> branch polkadot-v0.9.42 Dia-Oracle (oracle-pallet) -> branch polkadot-v0.9.42 Pendulum/Bifrost -> branch upgrade-v0.9.42.

Updates of the Storage Version

Following what was defined in this issue and in this notion page, we define in this PR the manual set of the storage version for all pallets which are behind the current version.

The code is added to the Executive type and will be used only for the purpose of this update.

For pallets vesting and transaction-payment we need to make custom migrations that make use of private definitions of the StorageVersion type and the Releases enum. This temporary substrate fork contains these migration types. here and here.

Changes in runtime

General

New TokenError variants were added to ChainExtensionTokenError enum and From<> functions.

Pallet Balances

In all configs we must add the following new types (FreezeIdentifier, MaxFreezes, MaxHolds and HoldIdentifier). Since we currently do not use any of these functionalities, we can pass the default configuration. We need to adjust if we require more than one freeze or hold.

 type FreezeIdentifier = ();
  type MaxFreezes = ();
  type MaxHolds = ConstU32<1>;
  type HoldIdentifier = RuntimeHoldReason;

Also in pallet_balances the DustRemoval trait bound has changed. Before it was required: type DustRemoval: OnUnbalanced<NegativeImbalance<Self, I>>; Which pallet Treasury implemented.

Passing type Treasure is not supported anymore since the bound is now: type DustRemoval: OnUnbalanced<CreditOf<Self, I>>; which is not implemented by the Treasury pallet, so we must define a new implementation. This is adopted from ComposableFi yet the functionality is the same which will transfer to the Treasury's account the unbalanced currency.

Pallet Contracts

Pallet Collective

ebma commented 8 months ago

Do we also want to fix all issues on Foucoco or do we ignore it because it's just our test network? I can see that at least the transaction-payment version is also on V1Ancient on Foucoco, I didn't check all other pallets though.

gianfra-t commented 8 months ago

I think we should also fix the issues on Foucoco in case there are. But should we do this in this PR?

gianfra-t commented 8 months ago

Okay so I looked at Foucoco and it seems like a good idea to bump here the versions that are behind, since they don't require much analysis.

Here is a brief breakdown:

ebma commented 7 months ago

@gianfra-t can you please resolve the conflicts of this PR? We need to prepare it for merging again

gianfra-t commented 7 months ago

The new fee per second implementation was merged into this branch, which removes the need for a temporary workaround used before.

prayagd commented 7 months ago

@ebma @gianfra-t would this update break the ledger app?

gianfra-t commented 7 months ago

I don't think so, because the extrinsics are not changing by the update (the transaction version should stay the same).

gianfra-t commented 6 months ago

Regarding merging this after https://github.com/pendulum-chain/pendulum/pull/410, do we plan to have both changes on the same runtime upgrade? I particularly have no strong opinion against doing both at the same time, and it could save some time. Also regarding this slack discussion.

ebma commented 6 months ago

Let's see if we can merge this and then this tomorrow so that we can link to a commit that is contained on the main branch of Spacewalk.

ebma commented 6 months ago

Oh just one more thing. I noticed that the PR includes the changes of https://github.com/pendulum-chain/pendulum/pull/432 which we rolled back in order not to mess with the XCM configuration until https://github.com/pendulum-chain/tasks/issues/202 is fully ready and implemented. I propose we roll those changes back here as well. I assume this is quite simple and can be done with a simple revert of the specific commit like was done in https://github.com/pendulum-chain/pendulum/pull/431.

gianfra-t commented 6 months ago

Oh, good that you remembered that @ebma, I completely forgot. Sadly I don't know if it will be that simple, mostly because there were some changes required for the old trader given the new version. So it will probably not look exactly like before.

gianfra-t commented 6 months ago

@ebma I reverted the merge with fee per second, which re-introduces this workaround for the TakeFirstAssetTrader. When using 0.9.42, simply using Tokens pallet instead of this ConcreteAsset struct defined in the workaround wont work, since it does not implement (anymore) the necessary traits. That I know, we only use the method minimum_balance from the inspect trait in TakeFirstAssetTrader. I removed the todo()! macro just to be safe so many implementations won't make sense.

Let me know what you think. I am testing with chopsticks right now.