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

add(mining): Restore internal miner #9311

Open
wants to merge 2 commits into
base: update-ecc-deps
Choose a base branch
from

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Feb 28, 2025

Motivation

We want to restore the experimental internal Testnet miner to run a Zebra-only Testnet with a published release.

Refresh of #8906.
Closes #8183.

Solution

  • Restores the internal miner code that was reverted last year

Tests

Manually tested and experimental

Follow-up Work

PR Checklist

  • The PR name is suitable for the release notes.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.
  • If the PR shouldn't be in the release notes, it has the
    C-exclude-from-changelog label.

@arya2 arya2 added C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces P-Medium ⚡ labels Feb 28, 2025
@arya2 arya2 self-assigned this Feb 28, 2025
@arya2 arya2 requested review from a team as code owners February 28, 2025 23:10
@arya2 arya2 requested review from upbqdn and removed request for a team February 28, 2025 23:10
@mpguerra mpguerra linked an issue Mar 4, 2025 that may be closed by this pull request
@mpguerra mpguerra removed the request for review from a team March 6, 2025 11:25
Copy link
Member

@upbqdn upbqdn left a comment

Choose a reason for hiding this comment

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

Should we go with a newer config version instead of zebrad/tests/common/configs/v1.9.0-internal-miner.toml?

Comment on lines +168 to +169
// If we got any solutions, try submitting them, because the new template might just
// contain some extra transactions. Mining extra transactions is optional.
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment related to the code?

Suggested change
// If we got any solutions, try submitting them, because the new template might just
// contain some extra transactions. Mining extra transactions is optional.

// Restore the code when conditions are met. https://github.com/ZcashFoundation/zebra/issues/8183
/// Returns `true` if the `nonce` and `solution` in `header` meet the difficulty threshold.
///
/// Assumes that the difficulty threshold in the header is valid.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Assumes that the difficulty threshold in the header is valid.
/// # Panics
///
/// - If `header` contains an invalid difficulty threshold.

@mpguerra
Copy link
Contributor

@mergify requeue

Copy link
Contributor

mergify bot commented Mar 18, 2025

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@mpguerra
Copy link
Contributor

@mergify refresh

Copy link
Contributor

mergify bot commented Mar 18, 2025

refresh

✅ Pull request refreshed

@gustavovalverde
Copy link
Member

@mergify rebase

Copy link
Contributor

mergify bot commented Mar 18, 2025

rebase

✅ Nothing to do for rebase action

@gustavovalverde
Copy link
Member

@arya2 you'll have to rebase this one manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-testing Category: These are tests P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restore internal miner
4 participants