Skip to content

fix: removed txs from test wallet#1139

Merged
glevco merged 1 commit intomasterfrom
fix/wallet-removed-tx
Oct 4, 2024
Merged

fix: removed txs from test wallet#1139
glevco merged 1 commit intomasterfrom
fix/wallet-removed-tx

Conversation

@glevco
Copy link
Contributor

@glevco glevco commented Sep 30, 2024

Motivation

We noticed that the wallet.get_balance_per_address() method was not working correctly in the end of the reward lock event simulation — it was not considering the removed tx. This PR fixes this.

Acceptance Criteria

  • Add voidance assertion in _remove_transactions().
  • Prevent event processing from reloading metadata.
  • Add CONSENSUS_TX_REMOVED event handling on test wallet.
  • Add typing to test_reward_lock.py.
  • Add balance and voidance assertions in reward lock test and event simulation scenario.

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 Sep 30, 2024
@glevco glevco self-assigned this Sep 30, 2024
@github-actions
Copy link

github-actions bot commented Sep 30, 2024

🐰 Bencher Report

Branchfix/wallet-removed-tx
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)
Lower Boundary
nanoseconds (ns)
(Limit %)
Upper Boundary
nanoseconds (ns)
(Limit %)
sync-v2 (up to 20000 blocks)📈 view plot
🚷 view threshold
101,446,853,776.88
(-0.54%)
91,802,408,704.46
(90.49%)
112,202,943,972.12
(90.41%)
🐰 View full continuous benchmarking report in Bencher

@codecov
Copy link

codecov bot commented Sep 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.29%. Comparing base (115569a) to head (067de83).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1139      +/-   ##
==========================================
- Coverage   84.40%   84.29%   -0.12%     
==========================================
  Files         317      317              
  Lines       24297    24303       +6     
  Branches     3697     3698       +1     
==========================================
- Hits        20508    20485      -23     
- Misses       3072     3091      +19     
- Partials      717      727      +10     

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

@glevco glevco marked this pull request as ready for review October 1, 2024 14:03
msbrogli
msbrogli previously approved these changes Oct 2, 2024
jansegre
jansegre previously approved these changes Oct 3, 2024
msbrogli
msbrogli previously approved these changes Oct 3, 2024
@glevco glevco dismissed stale reviews from msbrogli and jansegre via 34e7d9d October 3, 2024 21:03
@glevco glevco force-pushed the fix/wallet-removed-tx branch from 6b95cb0 to 34e7d9d Compare October 3, 2024 21:03
@glevco glevco force-pushed the fix/wallet-removed-tx branch from 34e7d9d to 69870b1 Compare October 3, 2024 21:05
@glevco glevco force-pushed the fix/wallet-removed-tx branch from 69870b1 to 067de83 Compare October 3, 2024 22:27
@glevco glevco merged commit e3b73dd into master Oct 4, 2024
@glevco glevco deleted the fix/wallet-removed-tx branch October 4, 2024 15:14
@jansegre jansegre mentioned this pull request Oct 4, 2024
2 tasks
@jansegre jansegre mentioned this pull request Dec 11, 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