Skip to content

feat(mergedmining): support dummy mining on coordinator#966

Merged
jansegre merged 1 commit intomasterfrom
feat/mm-coordinator-dummy
Aug 14, 2024
Merged

feat(mergedmining): support dummy mining on coordinator#966
jansegre merged 1 commit intomasterfrom
feat/mm-coordinator-dummy

Conversation

@jansegre
Copy link
Member

@jansegre jansegre commented Mar 4, 2024

Motivation

On the mainnet miners mostly use pools with custom integrations, but on the testnet the whole stack is in our control. For testing the newly activated (on the testnet) extended merkle_path len, we benefit from having the merged mining coordinator be able to do it seamlessly with some injected dummy merkle path.

Acceptance Criteria

  • Add --x-dummy-merged-mining and --x-dummy-merkle-len to run_merged_mining subcommand, for running a coordinator with a dummy chain (instead of BTC), and with a dummy merkle path with the specified length;
  • Adapt the merged mining coordinator to support mining without a BTC node and with dummy data instead.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@codecov
Copy link

codecov bot commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 52.94118% with 16 lines in your changes missing coverage. Please review.

Project coverage is 84.99%. Comparing base (e61463e) to head (f1b24cd).

Files Patch % Lines
hathor/merged_mining/coordinator.py 54.54% 11 Missing and 4 partials ⚠️
hathor/client.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #966      +/-   ##
==========================================
- Coverage   85.08%   84.99%   -0.10%     
==========================================
  Files         314      314              
  Lines       23919    23939      +20     
  Branches     3614     3621       +7     
==========================================
- Hits        20351    20346       -5     
- Misses       2863     2880      +17     
- Partials      705      713       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

glevco
glevco previously approved these changes Apr 2, 2024
Copy link
Contributor

@glevco glevco left a comment

Choose a reason for hiding this comment

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

I'm just wondering if this couldn't be simpler, for example, couldn't we just have a way to create a merge mined block and resolve it with the CPU miner? I mean, instead of having to start the coordinator, create a job, etc

@jansegre jansegre force-pushed the feat/mm-coordinator-dummy branch from 9a3a1e4 to e48581d Compare April 11, 2024 16:40
@jansegre jansegre marked this pull request as ready for review April 11, 2024 16:46
@jansegre jansegre requested a review from msbrogli as a code owner April 11, 2024 16:46
@jansegre jansegre force-pushed the feat/mm-coordinator-dummy branch from e48581d to 07d2a63 Compare April 11, 2024 16:47
@jansegre
Copy link
Member Author

I'm just wondering if this couldn't be simpler, for example, couldn't we just have a way to create a merge mined block and resolve it with the CPU miner? I mean, instead of having to start the coordinator, create a job, etc

It definitely can be made simpler mining on CPU, however it's hard to make it in real-time to mine an actual block what will be accepted and used by the rest of the network. Which there wouldn't be anything wrong with a block that is voided for being old, but it'd be better if we have a more realistic test where blocks are accepted. Also doing it in the coordinator makes stratum clients mine blocks with the extended merkle path, which is good to confirm it works with no special changes.

@jansegre jansegre requested a review from msbrogli May 3, 2024 14:29
@jansegre jansegre force-pushed the feat/mm-coordinator-dummy branch from 07d2a63 to 1a0c9db Compare May 3, 2024 14:29
@jansegre jansegre requested a review from glevco May 3, 2024 14:29
glevco
glevco previously approved these changes May 7, 2024
@jansegre jansegre force-pushed the feat/mm-coordinator-dummy branch from 1a0c9db to 1f7facd Compare June 10, 2024 13:09
@jansegre jansegre force-pushed the feat/mm-coordinator-dummy branch from 1f7facd to f1b24cd Compare July 26, 2024 16:36
msbrogli
msbrogli previously approved these changes Aug 9, 2024
@jansegre jansegre dismissed stale reviews from msbrogli and glevco via 47473dd August 14, 2024 15:45
@jansegre jansegre force-pushed the feat/mm-coordinator-dummy branch from f1b24cd to 47473dd Compare August 14, 2024 15:45
@github-actions
Copy link

🐰Bencher

ReportWed, August 14, 2024 at 15:47:04 UTC
Projecthathor-core
Branchfeat/mm-coordinator-dummy
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)
Latency Lower Boundary
nanoseconds (ns) | (%)
Latency Upper Boundary
nanoseconds (ns) | (%)
sync-v2 (up to 20000 blocks)✅ (view plot)104,176,275,883.32 (-12.98%)95,769,881,797.87 (91.93%)143,654,822,696.81 (72.52%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

@jansegre jansegre merged commit 5972abc into master Aug 14, 2024
@jansegre jansegre deleted the feat/mm-coordinator-dummy branch August 14, 2024 16:33
@jansegre jansegre mentioned this pull request Oct 4, 2024
2 tasks
@jansegre jansegre mentioned this pull request Dec 11, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants