Skip to content

fix(reward-lock): save removed txs in the consensus context#1092

Merged
glevco merged 1 commit intomasterfrom
fix/reward-lock/update-context
Jul 26, 2024
Merged

fix(reward-lock): save removed txs in the consensus context#1092
glevco merged 1 commit intomasterfrom
fix/reward-lock/update-context

Conversation

@glevco
Copy link
Contributor

@glevco glevco commented Jul 24, 2024

Depends on #1091

Motivation

When transactions get removed from the mempool after a reorg makes them invalid, their dependencies' metadata is updated. This update was being done directly in the storage, instead of in the consensus' context. This PR fixes this.

Acceptance Criteria

  • Move remove_transactions() from the TransactionStorage to the Consensus, as it is only used there and shouldn't be used anywhere else.
    • It should be the same, except now it receives a context argument and saves metadata through it instead of directly in the storage.
  • Update event simulator tests accordingly, as now 3 extra events are generated (that were missing before).

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

@glevco glevco added the bug Something isn't working label Jul 24, 2024
@glevco glevco self-assigned this Jul 24, 2024
@glevco glevco force-pushed the fix/reward-lock/update-context branch from 39f229d to 730437f Compare July 24, 2024 19:51
@glevco glevco marked this pull request as ready for review July 24, 2024 19:55
@glevco glevco requested review from jansegre and msbrogli as code owners July 24, 2024 19:55
@codecov
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 96.96970% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.00%. Comparing base (e61463e) to head (640b99d).

Files Patch % Lines
hathor/consensus/consensus.py 96.87% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1092      +/-   ##
==========================================
- Coverage   85.08%   85.00%   -0.08%     
==========================================
  Files         314      314              
  Lines       23919    23922       +3     
  Branches     3614     3614              
==========================================
- Hits        20351    20335      -16     
- Misses       2863     2876      +13     
- Partials      705      711       +6     

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

jansegre
jansegre previously approved these changes Jul 24, 2024
Base automatically changed from tests/event-queue/reward-lock to master July 25, 2024 19:15
@glevco glevco dismissed jansegre’s stale review July 25, 2024 19:15

The base branch was changed.

msbrogli
msbrogli previously approved these changes Jul 25, 2024
@glevco glevco dismissed msbrogli’s stale review July 25, 2024 19:29

The merge-base changed after approval.

@glevco glevco force-pushed the fix/reward-lock/update-context branch 2 times, most recently from 74be114 to 2ae57ca Compare July 25, 2024 21:34
@jansegre jansegre mentioned this pull request Jul 25, 2024
2 tasks
@glevco glevco force-pushed the fix/reward-lock/update-context branch from 2ae57ca to 640b99d Compare July 26, 2024 17:08
@glevco glevco merged commit eb0ad92 into master Jul 26, 2024
@glevco glevco deleted the fix/reward-lock/update-context branch July 26, 2024 20:41
@jansegre jansegre mentioned this pull request Aug 1, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants