Skip to content

remove assumption that coinbase scriptSig prefix contains only BIP34#1461

Merged
plebhash merged 1 commit into
stratum-mining:mainfrom
plebhash:dont-assume-script-sig-bip34
Feb 25, 2025
Merged

remove assumption that coinbase scriptSig prefix contains only BIP34#1461
plebhash merged 1 commit into
stratum-mining:mainfrom
plebhash:dont-assume-script-sig-bip34

Conversation

@plebhash

@plebhash plebhash commented Feb 6, 2025

Copy link
Copy Markdown
Member

close #1460

builds on top of #1442

@plebhash plebhash force-pushed the dont-assume-script-sig-bip34 branch 4 times, most recently from b0f0257 to e59e1e7 Compare February 6, 2025 14:28
@codecov

codecov Bot commented Feb 6, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 18.28%. Comparing base (d09acd7) to head (55c01d2).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1461      +/-   ##
==========================================
- Coverage   18.34%   18.28%   -0.06%     
==========================================
  Files         127      127              
  Lines        9723     9716       -7     
==========================================
- Hits         1784     1777       -7     
  Misses       7939     7939              
Flag Coverage Δ
binary_codec_sv2-coverage 0.00% <ø> (ø)
binary_sv2-coverage 6.96% <ø> (ø)
bip32_derivation-coverage 0.00% <ø> (ø)
buffer_sv2-coverage 24.88% <ø> (ø)
codec_sv2-coverage 0.02% <ø> (ø)
common_messages_sv2-coverage 0.17% <ø> (ø)
const_sv2-coverage 0.00% <ø> (ø)
error_handling-coverage 0.00% <ø> (ø)
framing_sv2-coverage 0.37% <ø> (ø)
jd_client-coverage 0.42% <ø> (ø)
jd_server-coverage 13.07% <ø> (ø)
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.79% <ø> (ø)
protocols 23.89% <100.00%> (-0.11%) ⬇️
roles 7.73% <ø> (ø)
roles_logic_sv2-coverage 11.59% <100.00%> (-0.11%) ⬇️
sv2_ffi-coverage 0.00% <ø> (ø)
template_distribution_sv2-coverage 0.00% <ø> (ø)
translator_sv2-coverage 9.58% <ø> (ø)
utils ?
v1-coverage 3.11% <ø> (ø)

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.

Comment thread protocols/v2/roles-logic-sv2/src/job_creator.rs Outdated
@plebhash plebhash force-pushed the dont-assume-script-sig-bip34 branch from e59e1e7 to 5d5d480 Compare February 17, 2025 20:52
Comment thread protocols/v2/roles-logic-sv2/src/job_creator.rs
Comment thread protocols/v2/roles-logic-sv2/src/job_creator.rs Outdated
@plebhash plebhash force-pushed the dont-assume-script-sig-bip34 branch 3 times, most recently from eb08cfa to 9c7153e Compare February 19, 2025 02:32

@jbesraa jbesraa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
I think a test should be added to examine the behavior when there is additional data in the scriptsig prefix beside bip34 data

@Shourya742 Shourya742 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK. one minor nit.

Comment thread protocols/v2/roles-logic-sv2/src/job_creator.rs Outdated
@plebhash plebhash force-pushed the dont-assume-script-sig-bip34 branch from 9c7153e to 4a59d63 Compare February 25, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SRI should not assume that coinbase scriptSig prefix contains only BIP34

6 participants