Skip to content

fix(dag_builder): Fix generating two or more vertices with the same hash#1207

Merged
msbrogli merged 1 commit intomasterfrom
fix/dag-builder-hash-conflict
Jan 30, 2025
Merged

fix(dag_builder): Fix generating two or more vertices with the same hash#1207
msbrogli merged 1 commit intomasterfrom
fix/dag-builder-hash-conflict

Conversation

@msbrogli
Copy link
Member

@msbrogli msbrogli commented Jan 30, 2025

Motivation

If might happen that two different transactions have the same hash if their description is exactly the same. This PR fixes it by increasing the nonce.

Acceptance Criteria

  1. Assert that each transaction has a unique hash.
  2. Fix conflicts by increasing the nonce.
  3. Set a maximum of 100 attempts to fix conflicts. If it fails, an exception is raised.

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

@msbrogli msbrogli requested a review from jansegre as a code owner January 30, 2025 19:07
@msbrogli msbrogli self-assigned this Jan 30, 2025
@msbrogli msbrogli force-pushed the fix/dag-builder-hash-conflict branch from 5a402a8 to 8b02ba5 Compare January 30, 2025 19:13
@github-actions
Copy link

🐰 Bencher Report

Branchfix/dag-builder-hash-conflict
Testbedubuntu-22.04

🚨 1 Alert

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Lower Boundary
(Limit %)
sync-v2 (up to 20000 blocks)Latency
minutes (m)
📈 plot
🚨 alert (🔔)
🚷 threshold
1.48
(-10.41%)
1.48
(100.46%)
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 alert (🔔)
🚷 view threshold
1.48
(-10.41%)
1.48
(100.46%)
1.81
(81.44%)
🐰 View full continuous benchmarking report in Bencher

@codecov
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 83.65%. Comparing base (a3bf50c) to head (8b02ba5).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
hathor/dag_builder/vertex_exporter.py 75.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1207      +/-   ##
==========================================
+ Coverage   83.58%   83.65%   +0.07%     
==========================================
  Files         320      320              
  Lines       24551    24562      +11     
  Branches     3764     3767       +3     
==========================================
+ Hits        20520    20547      +27     
+ Misses       3267     3256      -11     
+ Partials      764      759       -5     

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

@msbrogli msbrogli merged commit 8b02ba5 into master Jan 30, 2025
9 of 10 checks passed
@msbrogli msbrogli deleted the fix/dag-builder-hash-conflict branch January 30, 2025 19:30
This was referenced Feb 3, 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