Skip to content

feat(nano): include header-id when generating a sighash#1478

Merged
jansegre merged 1 commit intomasterfrom
feat/nano-header-sighash-include-id
Oct 21, 2025
Merged

feat(nano): include header-id when generating a sighash#1478
jansegre merged 1 commit intomasterfrom
feat/nano-header-sighash-include-id

Conversation

@jansegre
Copy link
Member

@jansegre jansegre commented Oct 21, 2025

Motivation

When generating the sighash bytes for a nano-header the header-id is not included, but it is for a fee-header. This is an inconsistency, although there aren't any security implications.

Acceptance Criteria

  • Include the header-id in get_sighash_bytes (or rather, don't exclude it)

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

@jansegre jansegre self-assigned this Oct 21, 2025
@jansegre jansegre requested a review from msbrogli as a code owner October 21, 2025 17:28
@jansegre jansegre moved this from Todo to In Progress (WIP) in Hathor Network Oct 21, 2025
glevco
glevco previously approved these changes Oct 21, 2025
@github-actions
Copy link

github-actions bot commented Oct 21, 2025

🐰 Bencher Report

Branchfeat/nano-header-sighash-include-id
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
minutes (m)
(Result Δ%)
Lower Boundary
minutes (m)
(Limit %)
Upper Boundary
minutes (m)
(Limit %)
sync-v2 (up to 20000 blocks)📈 view plot
🚷 view threshold
1.71 m
(-1.29%)Baseline: 1.73 m
1.56 m
(91.18%)
2.08 m
(82.26%)
🐰 View full continuous benchmarking report in Bencher

@jansegre jansegre force-pushed the feat/nano-header-sighash-include-id branch from d397797 to a1ff5a2 Compare October 21, 2025 18:49
glevco
glevco previously approved these changes Oct 21, 2025
@jansegre jansegre force-pushed the feat/nano-header-sighash-include-id branch from a1ff5a2 to 033e2cd Compare October 21, 2025 18:57
@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.13%. Comparing base (607e0a3) to head (033e2cd).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1478      +/-   ##
==========================================
+ Coverage   86.11%   86.13%   +0.02%     
==========================================
  Files         437      437              
  Lines       33968    33966       -2     
  Branches     5315     5315              
==========================================
+ Hits        29251    29257       +6     
+ Misses       3677     3673       -4     
+ Partials     1040     1036       -4     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jansegre jansegre moved this from In Progress (WIP) to In Progress (Done) in Hathor Network Oct 21, 2025
@jansegre jansegre moved this from In Progress (Done) to In Review (WIP) in Hathor Network Oct 21, 2025
@jansegre jansegre merged commit f2f21f3 into master Oct 21, 2025
20 of 22 checks passed
@jansegre jansegre deleted the feat/nano-header-sighash-include-id branch October 21, 2025 21:16
@github-project-automation github-project-automation bot moved this from In Review (WIP) to Waiting to be deployed in Hathor Network Oct 21, 2025
This was referenced Oct 21, 2025
@jansegre jansegre moved this from Waiting to be deployed to Done in Hathor Network Oct 22, 2025
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