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.

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

Update to polkadot 1.10.0 #40

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Conversation

girazoki
Copy link
Collaborator

@girazoki girazoki commented Apr 26, 2024

Upgrades dependencies to 1.10.0. A few nitpicks to research:

  • whether we can wire SingleBlockMigrations to pallet-migrations. Possibly for another PR though, but I think we can implement the OnRUntimeUpgrade hook for a custom struct in pallet_migrations, that we could pass here, instead of directly implementing this hook for the pallet.
  • why tranfer_assets tests take all less weight now: For me weights are correct: in the precompile we are doing one read for getting the account_code_metadata and then we are recording an additional read in most of the tests. We are also charging1000 gas units in all precompile calls. transfer_assets by itself takes 100_000_000 so I think the weights make sense with respect to the values I obtain: 100_000_000 + 1_000 + 1 + 1

@girazoki
Copy link
Collaborator Author

@Agusrodri can you give me a bit more clarity on the weights obtained before?

@Agusrodri
Copy link
Contributor

@Agusrodri can you give me a bit more clarity on the weights obtained before?

@girazoki what you describe is correct. In all functions of the precompile, we charge 1000 units to account for some operational logics, and also in all functions except from transfer_assets_location, we charge a possible db read to account for the db access that the generic implementation of the converter might have.

Previously, when I run the tests for this precompile, I got 4000 units more compared to the changes in this PR. I'm not sure about the reason of this decrease, maybe some upstream changes are the cause?

In any case, I guess your changes make sense as you describe, so in my opinion they are more illustrative now.

Comment on lines +153 to +154
// TODO: Currently we use just the first core here, but for elastic scaling
// we iterate and build on all of the cores returned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: Currently we use just the first core here, but for elastic scaling
// we iterate and build on all of the cores returned.
// TODO: Currently we use just the first core here, but for elastic scaling
// we iterate and build on all of the cores returned.
// More info: https://github.com/paritytech/polkadot-sdk/issues/1829

@girazoki girazoki merged commit 7295a81 into main Apr 29, 2024
10 checks passed
@girazoki girazoki deleted the girazoki-upgdate-to-polkadot-1.10.0 branch April 29, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants