Skip to content

feat: experimental-monitor-multi-block support#955

Merged
niklasad1 merged 77 commits intomainfrom
na-feature-gated-exp-miner
Mar 7, 2025
Merged

feat: experimental-monitor-multi-block support#955
niklasad1 merged 77 commits intomainfrom
na-feature-gated-exp-miner

Conversation

@niklasad1
Copy link
Copy Markdown
Contributor

@niklasad1 niklasad1 commented Jan 9, 2025

This PR adds support for the multi-block pallet which is added as new monitor command experimental-monitor-multi-block

Thus, the "old miner a.k.a single-block miner" isn't modified and should work as it did before.

To run the experimental multi-block command can be done as follows:

polkadot-staking-miner --uri ws://127.0.0.1:9966 experimental-monitor-multi-block --seed-or-path //Alice

Manual testing done:

Things to improve (possibly after merging):

  • Proper integration test against the parachain assethub-next
  • Make the NPoS algorithm configurable
  • Prometheus metrics
  • Double check hardcoding values (worth looking into whether we can utilize metadata v16 to improve that)
  • Check verification after pages submitted
  • Currently the code only checks that no better scores exist on chain and that the miner hasn't submitted yet.
    This is not sufficient because we must also make sure that all pages have been submitted if requested.
    AFAIU, otherwise the miner may be slashed

Extra notes for reviewer, lots of code is changed due to refactoring but the most important parts is:

  • commands/multi-block/monitor.rs: which contains the logic how the snapshot/signed phase are handled.

@niklasad1 niklasad1 assigned niklasad1 and unassigned kianenigma Feb 3, 2025
@niklasad1 niklasad1 requested a review from jsdw March 4, 2025 11:44
}

#[tokio::test]
#[ignore]
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.

fix before merge

@kianenigma
Copy link
Copy Markdown
Contributor

kianenigma commented Mar 5, 2025

Some todos:

  • You can use SubmissionMetadata to know if you have submitted
  • You can fetch the snapshot at any time, no need to sync with inner in Phase::Snapshot(inner).
  • Atm we are bailing too much, if half finished, complete it
  • Once round is over, if our submission is not the best, do the refund via clear_old_round_data.

@niklasad1 niklasad1 changed the title feat: support for multi-block miner feat: experimental-monitor-multi-block support Mar 7, 2025
@niklasad1 niklasad1 merged commit d4b9b1c into main Mar 7, 2025
10 checks passed
@niklasad1 niklasad1 deleted the na-feature-gated-exp-miner branch March 7, 2025 16:58
@miss-stake miss-stake moved this from In progress to Audited in Security Audit (PRs) - SRLabs Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Audited

Development

Successfully merging this pull request may close these issues.

6 participants