Add TemplateProvider::rpc_info by jbesraa · Pull Request #1429 · stratum-mining/stratum

jbesraa · 2025-01-31T15:47:45Z

this is needed for start_jds which does not connect to sv2 port but through rpc for bitcoind

this function should be used in order to avoid #1418

maybe this should be tested like the following:

start tp
start jds
broadcast a tx through tp
check if jds mempool have the tx

the tricky part is (4) as we dont have access to jds mempool from ITF

codecov · 2025-01-31T16:07:18Z

Codecov Report

Attention: Patch coverage is 39.21569% with 31 lines in your changes missing coverage. Please review.

Project coverage is 18.27%. Comparing base (b60dc61) to head (fb9a683).
Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
roles/jd-server/src/lib/mod.rs	25.64%	29 Missing ⚠️
roles/jd-server/src/lib/mempool/mod.rs	80.00%	1 Missing ⚠️
roles/jd-server/src/lib/status.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1429      +/-   ##
==========================================
+ Coverage   18.05%   18.27%   +0.22%     
==========================================
  Files         127      127              
  Lines        9559     9738     +179     
==========================================
+ Hits         1726     1780      +54     
- Misses       7833     7958     +125

Flag	Coverage Δ
binary_codec_sv2-coverage	`0.00% <ø> (ø)`
binary_sv2-coverage	`6.95% <ø> (+<0.01%)`	⬆️
bip32_derivation-coverage	`0.00% <ø> (ø)`
buffer_sv2-coverage	`?`
codec_sv2-coverage	`0.02% <ø> (ø)`
common_messages_sv2-coverage	`0.17% <ø> (+<0.01%)`	⬆️
const_sv2-coverage	`0.00% <ø> (ø)`
error_handling-coverage	`0.00% <ø> (ø)`
framing_sv2-coverage	`0.37% <ø> (+<0.01%)`	⬆️
jd_client-coverage	`0.42% <ø> (ø)`
jd_server-coverage	`12.98% <39.21%> (+3.96%)`	⬆️
job_declaration_sv2-coverage	`0.00% <ø> (ø)`
key-utils-coverage	`2.38% <ø> (ø)`
mining-coverage	`3.15% <ø> (-0.03%)`	⬇️
mining_device-coverage	`0.00% <ø> (ø)`
mining_proxy_sv2-coverage	`0.82% <ø> (ø)`
noise_sv2-coverage	`5.78% <ø> (+<0.01%)`	⬆️
protocols	`23.89% <ø> (-0.04%)`	⬇️
roles	`7.71% <39.21%> (+1.28%)`	⬆️
roles_logic_sv2-coverage	`11.62% <ø> (-0.02%)`	⬇️
sv2_ffi-coverage	`0.00% <ø> (ø)`
template_distribution_sv2-coverage	`0.00% <ø> (ø)`
translator_sv2-coverage	`9.53% <ø> (ø)`
utils	`25.04% <ø> (ø)`
v1-coverage	`3.11% <ø> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

plebhash · 2025-01-31T16:58:46Z

        "tp_username".to_string(),
        "tp_password".to_string(),


we also need to set username and password on the conf object that goes into Node::with_conf()

and make them match these

otherwise, the RPC authentication will fail

should we consider adding some verification into start_jds?

that way, JDS will never start unless there is:

a bitcoin core RPC port available

the authentication parameters are correct

the fact that JDS doesn't have this kind of early verification is probably the reason why this issue went un-noticed until now

Added new commits

jbesraa · 2025-02-10T12:26:20Z

-        // taken. Consequentally new_block_receiver in JDsMempool::on_submit is never read, possibly
-        // reaching the channel bound. The new_block_sender is given as input to
-        // JobDeclarator::start()
-        if url.contains("http") {


This commit only removes this if

plebhash · 2025-02-10T16:02:31Z

looks like we need to fix Clippy CI here

plebhash

approving as this is an amazing progress in different parts of the code (more robust JDS and ITF)

but I think it's important to highlight that the current approach on ITF is using TP as an umbrella term for ANY bitcoin node, and this could potentially re-inforce some misconceptions

as clarified here #1418 (comment), in strict terms of Sv2 Protocol, JDS never participates in TDP, and therefore labeling the node it connects to as TP is conceptually misleading

but anyways, I don't think this is a blocker here, as adapting this PR to allow for more primitives for labeling of different bitcoin nodes would not necessarily be a trivial change (and a bit out of scope)

we already solved the fundamental problem from #1418 (replacing Sv2 port with RPC, tests, etc)

just taking note for future reference

plebhash · 2025-02-10T23:48:15Z

MG CI jds-receive-solution-while-processing-declared-job is failing deterministically:

     Running `target/debug/message_generator_sv2 ../../test/message-generator/test/jds-receive-solution-while-processing-declared-job/jds-receive-solution-while-processing-declared-job.json`

EXECUTING ../../test/message-generator/test/jds-receive-solution-while-processing-declared-job/jds-receive-solution-while-processing-declared-job.json

STD ERR:    Compiling jd_server v0.1.3 (/Users/plebhash/develop/stratum/roles/jd-server)
STD ERR:     Finished dev [optimized + debuginfo] target(s) in 6.44s
STD ERR:      Running `target/llvm-cov-target/debug/jd_server -c ../test/config/jds-receive-solution-while-processing-declared-job/jds-config.toml`
STD ERR: thread 'main' panicked at jd-server/src/main.rs:106:10:
STD ERR: Failed to start JDS: InvalidRPCUrl
STD ERR: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
STD ERR: error: process didn't exit successfully: `/Users/plebhash/.rustup/toolchains/1.75.0-aarch64-apple-darwin/bin/cargo run --manifest-path /Users/plebhash/develop/stratum/roles/Cargo.toml --target-dir /Users/plebhash/develop/stratum/roles/target/llvm-cov-target -p jd_server -- -c ../test/config/jds-receive-solution-while-processing-declared-job/jds-config.toml` (exit status: 101)
STD ERR:     Finished dev [unoptimized + debuginfo] target(s) in 0.06s
STD ERR:      Running `target/debug/message_generator_sv2 ../../test/message-generator/mock/jdc-mock-jds-receive-solution-while-processing-declared-job.json`
STD OUT: 
STD OUT: EXECUTING ../../test/message-generator/mock/jdc-mock-jds-receive-solution-while-processing-declared-job.json
STD OUT: 
STD ERR: thread 'main' panicked at src/main.rs:440:13:
STD ERR: TEST FAILED
PanicInfo {
    payload: Any { .. },
    message: Some(
        not yet implemented,
    ),
    location: Location {
        file: "src/external_commands.rs",
        line: 206,
        col: 41,
    },
    can_unwind: true,
    force_no_backtrace: false,
}
thread 'main' panicked at src/main.rs:440:13:
TEST FAILED
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
mg test failed

looks like we are triggering the newly introduced InvalidRPCUrl error, which is probably uncovering a way in which this MG test is actually incomplete, since it does not launch any RPC endpoint for JDS to connect to

so I can see two possible paths forward:

fix the MG test so that there's some bitcoind that allows JDS to establish a RPC connection and avoid the InvalidRPCUrl
prioritize CI migration: jds-receive-solution-while-processing-declared-job MG to Integration Test #1419 and migrate this test to an IT

option 1. feels like taking a steps backwards to unblock a step forward, so I'd lean towards 2., which actually feels like taking 2 steps forward

unfortunately #1419 is blocked by #1420, so we also need to fix a few other things to allow our progress here

jbesraa · 2025-02-11T07:37:10Z

MG CI jds-receive-solution-while-processing-declared-job is failing deterministically:

     Running `target/debug/message_generator_sv2 ../../test/message-generator/test/jds-receive-solution-while-processing-declared-job/jds-receive-solution-while-processing-declared-job.json`

EXECUTING ../../test/message-generator/test/jds-receive-solution-while-processing-declared-job/jds-receive-solution-while-processing-declared-job.json

STD ERR:    Compiling jd_server v0.1.3 (/Users/plebhash/develop/stratum/roles/jd-server)
STD ERR:     Finished dev [optimized + debuginfo] target(s) in 6.44s
STD ERR:      Running `target/llvm-cov-target/debug/jd_server -c ../test/config/jds-receive-solution-while-processing-declared-job/jds-config.toml`
STD ERR: thread 'main' panicked at jd-server/src/main.rs:106:10:
STD ERR: Failed to start JDS: InvalidRPCUrl
STD ERR: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
STD ERR: error: process didn't exit successfully: `/Users/plebhash/.rustup/toolchains/1.75.0-aarch64-apple-darwin/bin/cargo run --manifest-path /Users/plebhash/develop/stratum/roles/Cargo.toml --target-dir /Users/plebhash/develop/stratum/roles/target/llvm-cov-target -p jd_server -- -c ../test/config/jds-receive-solution-while-processing-declared-job/jds-config.toml` (exit status: 101)
STD ERR:     Finished dev [unoptimized + debuginfo] target(s) in 0.06s
STD ERR:      Running `target/debug/message_generator_sv2 ../../test/message-generator/mock/jdc-mock-jds-receive-solution-while-processing-declared-job.json`
STD OUT: 
STD OUT: EXECUTING ../../test/message-generator/mock/jdc-mock-jds-receive-solution-while-processing-declared-job.json
STD OUT: 
STD ERR: thread 'main' panicked at src/main.rs:440:13:
STD ERR: TEST FAILED
PanicInfo {
    payload: Any { .. },
    message: Some(
        not yet implemented,
    ),
    location: Location {
        file: "src/external_commands.rs",
        line: 206,
        col: 41,
    },
    can_unwind: true,
    force_no_backtrace: false,
}
thread 'main' panicked at src/main.rs:440:13:
TEST FAILED
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
mg test failed

looks like we are triggering the newly introduced InvalidRPCUrl error, which is probably uncovering a way in which this MG test is actually incomplete, since it does not launch any RPC endpoint for JDS to connect to

so I can see two possible paths forward:

1. fix the MG test so that there's some `bitcoind` that allows JDS to establish a RPC connection and avoid the `InvalidRPCUrl`

2. prioritize [CI migration:  `jds-receive-solution-while-processing-declared-job` MG to Integration Test #1419](https://github.com/stratum-mining/stratum/issues/1419) and migrate this test to an IT

option 1. feels like taking a steps backwards to unblock a step forward, so I'd lean towards 2., which actually feels like taking 2 steps forward

unfortunately #1419 is blocked by #1420, so we also need to fix a few other things to allow our progress here

(1) seemed like a quick solution as it is just not including the bitcoind rpc data ccc2b52 , it seems to be ok now https://github.com/stratum-mining/stratum/actions/runs/13257911698/job/37007907249?pr=1429

jbesraa · 2025-02-11T08:13:02Z

Added new commit to resolve #1420 here 4962b9b. As mentioned in the test, JDC waits for for a downstream connection before starting to exchange messages with TP. not sure if this is a feature or a bug. might need further investigation.

plebhash · 2025-02-11T20:28:57Z

oops, looks like this is still not ready to be merged

CI now consistently breaking at jdc_tp_success_setup

https://github.com/stratum-mining/stratum/actions/runs/13270694992/job/37050470055?pr=1429

plebhash · 2025-02-11T20:29:09Z

+// Note that jd-client starts to exchange messages with the Template Provider after it has accepted
+// a downstream connection.
+#[tokio::test]
+async fn jdc_tp_success_setup() {


I don't think it's a good idea to add this test on this PR

this new test will block this PR from being merged due to CI

and this is blocked by #1420

It seems to be failing only on macos

jbesraa · 2025-02-16T13:59:16Z

Rebased without further changes

pavlenex · 2025-02-16T23:04:04Z

Hey @jbesraa, your PR cannot be rebased due to conflicts. Could you resolve them, please?

plebhash · 2025-02-16T23:04:54Z

pav bot 🤌

.. to return rpc info for outside connection

.. Used in JDS to validate RPC input contains a full URL including the scheme. Added to utility as it can be used by other crates.

- RPC client - JdsMempool RPC client simply tries to fetch `getblockchaininfo` request to validate it is able to establish a connection with the node. JdsMempool only calls `rpc_client.helath`.

.. Validate template provider RPC URL before initializing JDS. + Check mempool connection health before starting JDS

..Too verbose, code/test title are quite readable anyway

pavlenex · 2025-02-17T14:10:07Z

@plebhash @GitGab19 while bot above is cool, perhaps in the future we should consider detaching it from my personal account and have it under an account labeled as bot. I presume it can be confusing that a bot replies from my account for contributors who don't have context, now a high-priority but would be good to tackle it as we try to improve dev process structure.

plebhash reviewed Jan 31, 2025

View reviewed changes

jbesraa commented Feb 10, 2025

View reviewed changes

jbesraa changed the title ~~Add TemplateProvider::rpc_url~~ Add TemplateProvider::rpc_info Feb 10, 2025

plebhash approved these changes Feb 10, 2025

View reviewed changes

This was referenced Feb 10, 2025

CI migration: jds-receive-solution-while-processing-declared-job MG to Integration Test #1419

Closed

CI migration: MG to Integration/Unit Tests #1121

Closed

plebhash mentioned this pull request Feb 11, 2025

remove dbg! in roles logic #1469

Closed

plebhash added ready-to-be-merged triggers auto rebase bot and removed ready-to-be-merged triggers auto rebase bot labels Feb 11, 2025

plebhash reviewed Feb 11, 2025

View reviewed changes

plebhash added the ready-to-be-merged triggers auto rebase bot label Feb 16, 2025

jbesraa added 10 commits February 17, 2025 08:30

Add TemplateProvider::rpc_info

eb36e4e

.. to return rpc info for outside connection

Add common::url utility..

f8ab486

.. Used in JDS to validate RPC input contains a full URL including the scheme. Added to utility as it can be used by other crates.

Add health function to:

5f52bd7

- RPC client - JdsMempool RPC client simply tries to fetch `getblockchaininfo` request to validate it is able to establish a connection with the node. JdsMempool only calls `rpc_client.helath`.

Use JdsError for JDS

2829e82

Remove if as it is replaced with property validation

4d29299

Validate RPC URL in JDS

3fbb5f2

.. Validate template provider RPC URL before initializing JDS. + Check mempool connection health before starting JDS

Box error JdsError size as it is too big

67c0703

Point MG test to hosted bitcoind

9f3d2cc

Shrink test logs ..

9762d10

..Too verbose, code/test title are quite readable anyway

rm dbg!

fb9a683

plebhash mentioned this pull request Feb 17, 2025

replace pav bot with non-pav bot #1488

Closed

plebhash merged commit 410cbdc into stratum-mining:main Feb 17, 2025

plebhash mentioned this pull request Feb 24, 2025

JDS connects to wrong RPC port on Integration Tests #1418

Closed

Conversation

jbesraa commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

plebhash Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

plebhash Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbesraa Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

jbesraa Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

plebhash commented Feb 10, 2025

Uh oh!

plebhash left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plebhash commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbesraa commented Feb 11, 2025

Uh oh!

jbesraa commented Feb 11, 2025

Uh oh!

plebhash commented Feb 11, 2025

Uh oh!

plebhash Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbesraa Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

jbesraa commented Feb 16, 2025

Uh oh!

pavlenex commented Feb 16, 2025

Uh oh!

plebhash commented Feb 16, 2025

Uh oh!

pavlenex commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jbesraa commented Jan 31, 2025 •

edited

Loading

codecov Bot commented Jan 31, 2025 •

edited

Loading

plebhash Jan 31, 2025 •

edited

Loading

plebhash left a comment •

edited

Loading

plebhash commented Feb 10, 2025 •

edited

Loading

plebhash Feb 11, 2025 •

edited

Loading

pavlenex commented Feb 17, 2025 •

edited

Loading