Skip to content

pallet epm: add TrimmingStatus to the mined solution#1659

Merged
niklasad1 merged 8 commits into
masterfrom
na-epm-trimming-api
Sep 22, 2023
Merged

pallet epm: add TrimmingStatus to the mined solution#1659
niklasad1 merged 8 commits into
masterfrom
na-epm-trimming-api

Conversation

@niklasad1

Copy link
Copy Markdown
Contributor

For tools such that is using the Miner it's useful to know whether a solution was trimmed or not and also how much that was trimmed.

For tools such that is using the `Miner` it's useful to know whether a
solution was trimmed or not.
@niklasad1 niklasad1 requested review from a team September 21, 2023 09:05
@niklasad1 niklasad1 changed the title pallet epm: add TrimmingStatus to solution pallet epm: add TrimmingStatus to the mined solution Sep 21, 2023
@niklasad1 niklasad1 added the T2-pallets This PR/Issue is related to a particular pallet. label Sep 21, 2023

/// Reports the trimming result of mined solution
#[derive(Debug, Clone)]
pub struct TrimmingStatus {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure about naming this, could also be TrimmingResult or something.

could also just be a boolean but I prefer a struct here

Comment thread substrate/frame/election-provider-multi-phase/src/unsigned.rs Outdated

@lexnv lexnv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! 👍 Small nit: would be nice to have a test that checks a non-trimmed and a trimmed solution to ensure the numbers are properly propagated. Not sure how easy that is to implement

Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>

@kianenigma kianenigma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please find one test that actually does trimming and assert on the output.

@niklasad1

Copy link
Copy Markdown
Contributor Author

bot merge

@command-bot

command-bot Bot commented Sep 22, 2023

Copy link
Copy Markdown

@niklasad1 bot merge and bot rebase are not supported anymore. Please use native Github "Auto-Merge" and "Update Branch" buttons instead.
image

@niklasad1 niklasad1 merged commit f79fa6c into master Sep 22, 2023
@niklasad1 niklasad1 deleted the na-epm-trimming-api branch September 22, 2023 17:22
ordian added a commit that referenced this pull request Sep 27, 2023
* master: (61 commits)
  OpenGov in Westend and Rococo (#1177)
  Associated type Hasher for `QueryPreimage`, `StorePreimage` and `Bounded` (#1720)
  Migrate polkadot-primitives to v6 (#1543)
  genesis-builder: implemented for all runtimes (#1492)
  `BlockId` removal: `tx-pool` refactor (#1678)
  Bump directories from 4.0.1 to 5.0.1 (#1656)
  Allow debug_assertions in short-benchmarks CI job (#1711)
  chainHead/storage: Fix storage iteration using the query key (#1665)
  Implement more useful traits in `Slot` type (#1595)
  Make downloads in parallel and give more time to complete (#1699)
  Bump actions/checkout from 4.0.0 to 4.1.0 (#1688)
  contracts: Fix incorrect storage alias in mirgration (#1687)
  Fix documentation about justification and `finalized == true` requirement (#1607)
  tweak pallet macro (genesis_config etc) to cater for RA users as well. (#1689)
  Uncoupling pallet-xcm from frame-system's RuntimeCall (#1684)
  Bump aes-gcm from 0.10.2 to 0.10.3 (#1681)
  docs / Update PR template to reflect monorepo (#1674)
  update contributing guide and ui-tests scripts (#1668)
  pallet epm: add `TrimmingStatus` to the mined solution (#1659)
  Update HRMP pallet benchmarking to use benchmarks v2 (#1676)
  ...
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
For tools such that is using the `Miner` it's useful to know whether a
solution was trimmed or not and also how much that was trimmed.

---------

Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
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.

4 participants