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

pallet epm: add TrimmingStatus to the mined solution #1659

Merged
merged 8 commits into from
Sep 22, 2023
Merged

pallet epm: add TrimmingStatus to the mined solution #1659

merged 8 commits into from
Sep 22, 2023

Conversation

niklasad1
Copy link
Member

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.

@niklasad1
pallet epm: add TrimmingStatus to solution
ed7f279 
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
niklasad1
niklasad1 commented Sep 21, 2023
View reviewed changes
substrate/frame/election-provider-multi-phase/src/unsigned.rs
@@ -108,6 +108,27 @@ impl From<FeasibilityError> for MinerError {
}
}

/// Reports the trimming result of mined solution
#[derive(Debug, Clone)]
pub struct TrimmingStatus {
Copy link
Member 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

lexnv
lexnv approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@lexnv lexnv left a comment

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

@niklasad1 @lexnv
Update substrate/frame/election-provider-multi-phase/src/unsigned.rs
8ac0450 
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
@niklasad1 niklasad1 mentioned this pull request Sep 21, 2023
Merged
kianenigma
kianenigma approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

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
Member Author

bot merge

@command-bot
Copy link

command-bot bot commented Sep 22, 2023

@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
6 checks passed
@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
@ordian
merge master and resolve conflicts (v8)
6fb4b7f 
* 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)
  ...
@kiltbot kiltbot mentioned this pull request Oct 19, 2023
Closed
@ahmadkaouk ahmadkaouk mentioned this pull request Nov 16, 2023
Closed
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@niklasad1 @lexnv
pallet epm: add TrimmingStatus to the mined solution (paritytech#1659)
8d1cd0c 
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>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 26, 2024
@bkontur
More accurate dispatch_result: true/false (paritytech#1659)
b892d11
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 27, 2024
@bkontur
More accurate dispatch_result: true/false (paritytech#1659)
eaef31f
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@bkontur
More accurate dispatch_result: true/false (paritytech#1659)
0748952
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
9070f96
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
488ca6f
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
700a5af
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
6fe8914
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
87f9c6f
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
7237ab0
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
7e7ec0c
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
60a6359
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
413ec7d
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
10b423d
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
501d207
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
@bkontur @serban300
More accurate dispatch_result: true/false (paritytech#1659)
a24ad1c
bkchr pushed a commit that referenced this pull request Apr 10, 2024
@bkontur @bkchr
More accurate dispatch_result: true/false (#1659)
56d58d6
This was referenced Jun 5, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lexnv lexnv lexnv approved these changes

@kianenigma kianenigma kianenigma approved these changes

@gpestana gpestana Awaiting requested review from gpestana

Assignees
No one assigned
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@niklasad1 @kianenigma @lexnv