Skip to content

fix: we should unspend transaction tx outputs when the tx spending it…#281

Merged
andreabadesso merged 30 commits intomasterfrom
fix/unspend-tx-outputs
Sep 16, 2025
Merged

fix: we should unspend transaction tx outputs when the tx spending it…#281
andreabadesso merged 30 commits intomasterfrom
fix/unspend-tx-outputs

Conversation

@andreabadesso
Copy link
Copy Markdown
Collaborator

@andreabadesso andreabadesso commented Aug 21, 2025

Acceptance Criteria

  • We should properly "unspend" utxos spent by voided transactions
  • We should "unlock" utxos that are locked by a given txproposal in case this txproposal fails.
  • We should properly recalculate wallet balances when a transaction is voided

IMPORTANT: In this PR we are (1) using a experimental docker image for the new tests and (2) skipping genesis address balance check

(1) This will be fixed when HathorNetwork/hathor-core#1392 is merged
(2) This is a race condition, tests pass when running individually, we must debug and fix it in a later PR

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
  • Make sure either the unit tests and/or the QA tests are capable of testing the new features
  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@andreabadesso andreabadesso force-pushed the fix/unspend-tx-outputs branch from f72d69f to 89fdbe1 Compare August 21, 2025 02:59
@andreabadesso andreabadesso force-pushed the fix/unspend-tx-outputs branch 2 times, most recently from 1aa8524 to 0e09722 Compare August 25, 2025 20:41
@andreabadesso andreabadesso self-assigned this Aug 27, 2025
@andreabadesso andreabadesso moved this from Todo to In Progress (WIP) in Hathor Network Aug 27, 2025
@andreabadesso andreabadesso force-pushed the fix/unspend-tx-outputs branch 13 times, most recently from 2211677 to e35c21b Compare September 9, 2025 13:01
r4mmer
r4mmer previously approved these changes Sep 12, 2025
tuliomir
tuliomir previously approved these changes Sep 12, 2025
@github-project-automation github-project-automation Bot moved this from In Progress (Done) to In Review (WIP) in Hathor Network Sep 12, 2025
@andreabadesso andreabadesso dismissed stale reviews from tuliomir and r4mmer via 13def6f September 12, 2025 18:12
tuliomir
tuliomir previously approved these changes Sep 12, 2025
tuliomir
tuliomir previously approved these changes Sep 12, 2025
r4mmer
r4mmer previously approved these changes Sep 12, 2025
@andreabadesso andreabadesso dismissed stale reviews from r4mmer and tuliomir via e6458b8 September 15, 2025 15:45
@andreabadesso
Copy link
Copy Markdown
Collaborator Author

andreabadesso commented Sep 15, 2025

e6458b8 fixes an issue with the integration tests causing them to consistently fail when running all tests at once but to succeed when ran individually, here is a better explanation:

Problem: The SyncMachine context was being shared across all test instances, causing the LRU cache (txCache) to persist between tests. This led to the unchanged guard skipping genesis transactions in subsequent tests because they were already cached from previous tests.

Root Cause:

  1. Test 1 processes genesis transaction -> stores it in LRU cache
  2. Test 2 receives same genesis transaction -> unchanged guard finds it in cache -> skips processing
  3. Result: genesis address has 0 balance instead of expected balance

Solution: Changed from shared context to dynamically created LRU cache per machine instance:

  • Before: txCache: new LRU(TX_CACHE_SIZE) (shared)
  • After: txCache: null -> initialized fresh in INITIALIZING state

More context on the unchanged guard:

This guard is a performance optimization that prevents redundant database operations by checking if transaction metadata has actually changed. It compares the cached hash of a transaction's metadata against the current event's metadata hash. If they match, the event is ignored since we've already processed this exact state.

The guard is designed to skip repeated VERTEX_METADATA_CHANGED events for transactions we've already seen - for example, if only the accumulated weight changes (which wallet-service doesn't track), we can safely ignore the event. However, when the cache persisted across tests, it incorrectly skipped legitimate first-time processing of genesis transactions in subsequent test scenarios.

@tuliomir tuliomir moved this from In Review (WIP) to In Review (Done) in Hathor Network Sep 15, 2025
@andreabadesso andreabadesso merged commit 3b8765b into master Sep 16, 2025
1 check passed
@github-project-automation github-project-automation Bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Sep 16, 2025
This was referenced Nov 17, 2025
@andreabadesso andreabadesso moved this from Waiting to be deployed to Done in Hathor Network Nov 24, 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.

4 participants