Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

1.1.0 Release #68

Closed
bkchr opened this issue Oct 19, 2023 · 34 comments
Closed

1.1.0 Release #68

bkchr opened this issue Oct 19, 2023 · 34 comments

Comments

@bkchr
Copy link
Contributor

bkchr commented Oct 19, 2023

Let's already start with the list of things to include for the 1.1.0 release. Please everyone leave a comment with a link to a pr or issue, so we have it tracked.

I would propose that we directly start with the release after we have finally finished 1.0.0.
@bkontur
Copy link
Contributor

bkontur commented Oct 19, 2023 •
edited by bkchr
Loading

@joepetrowski
Copy link
Contributor

joepetrowski commented Oct 20, 2023 •
edited
Loading

@acatangiu
Copy link
Contributor

acatangiu commented Oct 20, 2023 •
edited by bkchr
Loading

@bkchr
Copy link
Contributor Author

bkchr commented Nov 6, 2023

I would like that we slowly start with the release. I think the best would be to get @muharem's PR merged first: #56

Probably the best to keep this pr and then create another one that updates to 1.3.0.

@franciscoaguirre
Copy link
Contributor

When that PR is merged, I can make one updating to 1.3.0

@acatangiu
Copy link
Contributor

When that PR is merged, I can make one updating to 1.3.0

afaik @svyatonik is already working on this: svyatonik#2

@muharem
Copy link
Contributor

muharem commented Nov 8, 2023 •
edited
Loading

it would be nice to get this PR to the release #84
I can not base it on #56 since the branch in my fork

will we include polkadot-sdk#1.3 to the this release? @bkchr

@bkchr
Copy link
Contributor Author

bkchr commented Nov 9, 2023

will we include polkadot-sdk#1.3 to the this release? @bkchr

As I already said above, I would do it.

@muharem
Copy link
Contributor

muharem commented Nov 9, 2023

one more candidate, simple migration removal #54

@Sophia-Gold
Copy link

Sophia-Gold commented Nov 16, 2023 •
edited
Loading

@muharem
Copy link
Contributor

muharem commented Nov 30, 2023 •
edited by bkchr
Loading

@bkchr
Copy link
Contributor Author

bkchr commented Dec 11, 2023

So, all the prs mentioned above are merged. I still want to include #117 and #118. Maybe also @Leemo94 also wants to bring up his small pr for changing the Kusama OpenGov tracks config. However, I would not wait on this if the pr is not popping up today.

Otherwise we still require a weights update. @ggwpez @liamaharon can you please do this?

@bkchr
Copy link
Contributor Author

bkchr commented Dec 11, 2023

#119 this one as well.

@acatangiu acatangiu mentioned this issue Dec 11, 2023
Merged
@NachoPal
Copy link
Contributor

About the weights, I found that this will provoke an Overweight message.

@acatangiu
Copy link
Contributor

acatangiu commented Dec 11, 2023 •
edited
Loading

FWIW: many (if not all) of the required benchmarks have already been run against (relatively) latest main in #108 - you could also get them from there

@NachoPal
Copy link
Contributor

NachoPal commented Dec 12, 2023 •
edited
Loading

Since the introduction of paritytech/polkadot-sdk#1234, the teleport when kicking a member will fail if the AlliancePalletAccount does not have some extra balance to pay the fees.

AlliancePalletAccount (13UVJyLnbVp6r4JgFxb88FAvQBL5WCdE2FU8uzyCJCHkDhx1) should be funded to pay the fees or otherwise it will fail in production.

Similarly will happen with ReferendaPalletAccount (13UVJyLnbVp9974YNNtrNWNjJQhBYSnMoTQycaARVAt49q7h).

@joepetrowski
Copy link
Contributor

We are planning to just add an UnpaidTeleport instruction to XCMv4.

@bkontur
Copy link
Contributor

bkontur commented Dec 12, 2023

About the weights, I found that this will provoke an Overweight message.

@NachoPal Do you have any specific scenario in mind?

Actually, relay chains and system parachains do not recognize any trusted reserves type IsReserve = ();, so they cannot process ReserveAssetDeposited. When we regenerate new benchmarks, then this 500_000_000_000 will become 18_446_744_073_709_551_000 (as a Weight::MAX), this is ok since Gav removed remote weight estimation here. If you want to process ReserveAssetDeposited, you need to set up correct benchmark and IsReserve, e.g. as we did for bridges here.

@acatangiu
Copy link
Contributor

What about #108? It's reviewed and ready to merge IMO and only adds functionality that needs further governance actions to actually enable.

It'd be great to get it in this upcoming release!

@muharem
Copy link
Contributor

muharem commented Dec 12, 2023

we might want to include this small PR, one constant value change, otherwise the current salary budget limit, might not be enough

@NachoPal
Copy link
Contributor

NachoPal commented Dec 12, 2023 •
edited
Loading

StakingAdmin, FellowshipAdmin, etc.. Origins should be waived from paying fees, otherwise it will fail when trying to send xcm messages.

Talked to @muharem and he'll open a PR

@bkchr
Copy link
Contributor Author

bkchr commented Dec 13, 2023

What about #108? It's reviewed and ready to merge IMO and only adds functionality that needs further governance actions to actually enable.

It'd be great to get it in this upcoming release!

Still misses reviews.

@bkontur bkontur mentioned this issue Dec 13, 2023
Merged
4 tasks
@mustermeiszer
Copy link
Contributor

@bkchr will this also include #67 automatically?

@joepetrowski
Copy link
Contributor

Yes

@muharem
Copy link
Contributor

muharem commented Dec 18, 2023

Since the introduction of paritytech/polkadot-sdk#1234, the teleport when kicking a member will fail if the AlliancePalletAccount does not have some extra balance to pay the fees.

AlliancePalletAccount (13UVJyLnbVp6r4JgFxb88FAvQBL5WCdE2FU8uzyCJCHkDhx1) should be funded to pay the fees or otherwise it will fail in production.

Similarly will happen with ReferendaPalletAccount (13UVJyLnbVp9974YNNtrNWNjJQhBYSnMoTQycaARVAt49q7h).

fixed with this PR - #125

@liamaharon liamaharon mentioned this issue Dec 19, 2023
Merged
9 tasks
@NachoPal
Copy link
Contributor

StakingAdmin, FellowshipAdmin, etc.. Origins should be waived from paying fees, otherwise it will fail when trying to send xcm messages.

This was fixed also in #125

@bkchr
Copy link
Contributor Author

bkchr commented Dec 22, 2023 •
edited by ggwpez
Loading

The following prs are still missing:

fellowship-merge-bot bot pushed a commit that referenced this issue Jan 5, 2024
@liamaharon @ggwpez
@joepetrowski @acatangiu
Update Runtime Weights (#129)
48ccfae 
For
#68 (comment).

Uses instructions introduced to the readme in
#127 to re-bench all
runtimes.

TODO
- [x] Is this much change % expected? Is benchmarking on vCPUs flawed
because they are arbitrarily shared with other cloud customers?
- It seems to be fine, the changes align with actual changes in
configuration of the runtimes (thanks @ggwpez!).
- [x] Polkadot
- [x] Kusama
- [x] Asset Hub Polkadot
- [x] Asset Hub Kusama
- [x] Bridge Hub Polkadot
- [x] Bridge Hub Kusama
- [x] Collectives Polkadot
- [ ] Look into why subweight shows some errors

## Polkadot Changes

<img width="1085" alt="Screenshot 2023-12-19 at 11 06 20"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/272e2ecd-48da-4ebb-b6e2-ff9fd73683fe">
<img width="1085" alt="Screenshot 2023-12-19 at 11 08 24"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/a37dea2e-abf6-4ad8-b3ef-abed6cb3e955">
<img width="1085" alt="Screenshot 2023-12-19 at 11 10 14"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/08788059-a052-4af6-b2aa-c791a3cddc24">

## Kusama Changes

<img width="1074" alt="Screenshot 2023-12-19 at 11 12 19"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/b096d1ec-384f-436f-ae9c-f6f929d14b9b">
<img width="1074" alt="Screenshot 2023-12-19 at 11 12 43"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/1bc757e8-c00c-4cf0-bea6-f74b92a33355">

## Asset Hub Polkadot Changes

<img width="1313" alt="Screenshot 2023-12-19 at 20 34 25"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/ea658d57-365d-45de-b0be-2c4627783e64">

## Asset Hub Kusama Changes

<img width="1313" alt="Screenshot 2023-12-19 at 20 43 37"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/963483ee-0d9f-49dc-9a83-31b443df1f10">

## Bridge Hub Polkadot

<img width="1313" alt="Screenshot 2023-12-20 at 11 49 13"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/0da77515-bb36-4a41-9c88-463ea28d9a9e">


## Bridge Hub Kusama

<img width="1313" alt="Screenshot 2023-12-20 at 11 48 39"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/97c7efa4-6a22-42fe-90e0-8477e0628d00">

## Collectives Polkadot

<img width="1369" alt="Screenshot 2023-12-20 at 11 50 33"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/6b2b22f3-fc87-4f19-a126-8d4be2335aa6">

<img width="1369" alt="Screenshot 2023-12-20 at 11 51 07"
src="https://github.com/polkadot-fellows/runtimes/assets/16665596/168ec1e2-ec47-4213-8899-f2d51c8ec6d1">

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: joe petrowski <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
@ggwpez
Copy link
Member

ggwpez commented Jan 9, 2024

All ticked off now, can we go ahead?

@joepetrowski
Copy link
Contributor

All ticked off now, can we go ahead?

@NachoPal was doing a final run of integration tests. Any ETA on them finishing?

@NachoPal
Copy link
Contributor

NachoPal commented Jan 9, 2024 •
edited
Loading

@NachoPal was doing a final run of integration tests. Any ETA on them finishing?

All green, but spec_versions still need to be bumped.

@joepetrowski
Copy link
Contributor

#138

@NachoPal
Copy link
Contributor

NachoPal commented Jan 11, 2024 •
edited
Loading

About the weights, I found that this will provoke an Overweight message.

@NachoPal Do you have any specific scenario in mind?

Actually, relay chains and system parachains do not recognize any trusted reserves type IsReserve = ();, so they cannot process ReserveAssetDeposited. When we regenerate new benchmarks, then this 500_000_000_000 will become 18_446_744_073_709_551_000 (as a Weight::MAX), this is ok since Gav removed remote weight estimation here. If you want to process ReserveAssetDeposited, you need to set up correct benchmark and IsReserve, e.g. as we did for bridges here.

Yes, you are right, that message should fail, but it should fail with an UntrustedReserveLocation and not with an Overweight. It is properly working now.

@bkontur
Copy link
Contributor

bkontur commented Jan 11, 2024

About the weights, I found that this will provoke an Overweight message.

@NachoPal Do you have any specific scenario in mind?
Actually, relay chains and system parachains do not recognize any trusted reserves type IsReserve = ();, so they cannot process ReserveAssetDeposited. When we regenerate new benchmarks, then this 500_000_000_000 will become 18_446_744_073_709_551_000 (as a Weight::MAX), this is ok since Gav removed remote weight estimation here. If you want to process ReserveAssetDeposited, you need to set up correct benchmark and IsReserve, e.g. as we did for bridges here.

Yes, you are right, that message should fail, but it should fail with an UntrustedReserveLocation and not with an Overweight. It is properly working now.

we had a similar discussions with @acatangiu around here here

hmm, maybe, if we know that IsReserve = () is no-op, than the benchmark should set Weight::zero() instead of T::BlockWeights::get().max_block or Weight::MAX.
But it could be risky, e.g. if we change type IsReseve = ... and forget to regenerate benchmarks...

I don't know, what is the best solution here. Regardless of returned xcm error (WeightNotComputable vs. UntrustedReserveLocation), Weight::MAX seems safer.

@bkchr
Copy link
Contributor Author

bkchr commented Jan 11, 2024

Going to close the issue, as the release is finished.

@bkchr bkchr closed this as completed Jan 11, 2024
@bkontur bkontur mentioned this issue Mar 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@franciscoaguirre @bkchr @muharem @bkontur @ggwpez @NachoPal @Sophia-Gold @joepetrowski @mustermeiszer @acatangiu