Skip to content

Conversation

@dastansam
Copy link
Contributor

Problem

take() consumes only 1 read worth of weight in single-block-migrations example, while take() is get() + kill(), i.e should be 1 read + 1 write. I think this could mislead developers who follow this example to write their migrations

@dastansam dastansam requested a review from a team as a code owner April 25, 2024 22:32
@dastansam dastansam force-pushed the take-weight-correction branch from f4d6426 to 1dd84cf Compare April 25, 2024 22:32
@dastansam dastansam force-pushed the take-weight-correction branch from 1dd84cf to 0f0882e Compare April 25, 2024 22:33
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 1/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6065203

@liamaharon liamaharon added the T2-pallets This PR/Issue is related to a particular pallet. label Apr 26, 2024
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

I think it depends on the Trie version, but okay. Many chains are still on V0.

@bkchr bkchr enabled auto-merge May 12, 2024 22:05
@bkchr bkchr added this pull request to the merge queue May 12, 2024
Merged via the queue into paritytech:master with commit efc2132 May 12, 2024
@dastansam dastansam deleted the take-weight-correction branch May 13, 2024 07:53
ordian added a commit that referenced this pull request May 14, 2024
* master:
  improve MockValidationDataInherentDataProvider to support async backing (#4442)
  Bump `proc-macro-crate` to the latest version (#4409)
  [ci] Run check-runtime-migration in GHA (#4441)
  prospective-parachains rework (#4035)
  [ci] Add forklift to GHA ARC (#4372)
  `CheckWeight` SE: Check for extrinsic length + proof size combined (#4326)
  Add generate and verify logic for `AncestryProof` (#4430)
  Rococo AH: undeploy trie migration (#4414)
  Remove `substrate-frame-cli` (#4403)
  migrations: `take()`should consume read and write operation weight (#4302)
  `remote-externalities`: store block header in snapshot (#4349)
  xcm-emlator: Use `BlockNumberFor` instead of `parachains_common::BlockNumber=u32` (#4434)
  Remove `pallet::getter` usage from authority-discovery pallet (#4091)
  Remove pallet::getter usage from pallet-contracts-mock-network (#4417)
  Add docs to request_core_count (#4423)
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
…aritytech#4302)

#### Problem
`take()` consumes only 1 read worth of weight in
`single-block-migrations` example, while `take()`
[is](https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/support/src/storage/unhashed.rs#L63)
`get() + kill()`, i.e should be 1 read + 1 write. I think this could
mislead developers who follow this example to write their migrations

---------

Co-authored-by: Bastian Köcher <[email protected]>
liuchengxu pushed a commit to liuchengxu/polkadot-sdk that referenced this pull request Jun 19, 2024
…aritytech#4302)

#### Problem
`take()` consumes only 1 read worth of weight in
`single-block-migrations` example, while `take()`
[is](https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/support/src/storage/unhashed.rs#L63)
`get() + kill()`, i.e should be 1 read + 1 write. I think this could
mislead developers who follow this example to write their migrations

---------

Co-authored-by: Bastian Köcher <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…aritytech#4302)

#### Problem
`take()` consumes only 1 read worth of weight in
`single-block-migrations` example, while `take()`
[is](https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/support/src/storage/unhashed.rs#L63)
`get() + kill()`, i.e should be 1 read + 1 write. I think this could
mislead developers who follow this example to write their migrations

---------

Co-authored-by: Bastian Köcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T2-pallets This PR/Issue is related to a particular pallet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants