Skip to content

Extract mempool re-validation logic from CleanupWorker and test it - #2457

Open
Ergologica wants to merge 2 commits into
ergoplatform:masterfrom
Ergologica:fix/cleanup-worker-extract-1556
Open

Extract mempool re-validation logic from CleanupWorker and test it#2457
Ergologica wants to merge 2 commits into
ergoplatform:masterfrom
Ergologica:fix/cleanup-worker-extract-1556

Conversation

@Ergologica

Copy link
Copy Markdown

Closes #1556.

The change

CleanupWorker.validatePool was a private method of the actor, closing over nodeSettings, the system clock and a hard-coded cost limit, so the only way to reach it was to start an actor system and drive it through MempoolAuditor. MempoolAuditorSpec does exactly that, and as a result covers the re-validation logic only indirectly and only in its happy path.

The logic moves to object CleanupWorker as a function of its arguments:

def validatePool(validator: UtxoStateReader,
                 mempool: ErgoMemPoolReader,
                 maxTransactionCost: Int,
                 timeLimit: Long,
                 now: Long,
                 costLimit: Long = CostLimit): CleanupResult
  • now is passed in instead of calling System.currentTimeMillis(), so the staleness filter is testable without sleeping.
  • costLimit is a parameter defaulting to the same hard-coded CostLimit as before, so the cut-off can be exercised with small numbers.
  • The result is a named CleanupResult(validated, invalidated) rather than a bare tuple.
  • The staleness filter is also exposed on its own as transactionsToValidate, since it is the part with an off-by-one worth pinning.

The actor keeps its behaviour: it still runs the work in a Future, still sends RecheckedTransactions / EliminateTransactions to the node view holder, still replies CleanupDone, and still logs the same line at the same level. No behaviour change is intended anywhere.

Tests

New CleanupWorkerSpec, no actor system involved:

  • transactions checked recently enough are not re-checked at all;
  • transactions stale enough are, including the boundary — exactly timeLimit old is not stale yet, > and not >=;
  • still-valid transactions come back in validated with lastCost filled in, having been None before;
  • transactions spending inputs that do not exist come back in invalidated;
  • a mixed pool splits correctly between the two;
  • the cost limit stops the loop — with costLimit = 1 exactly one transaction is processed, since costAcc starts at zero, and with no limit the whole pool is.

I deliberately kept the diff to two files: #1560 grew to 25 files, which I suspect is part of why it stalled.

sbt "testOnly org.ergoplatform.local.*" — 9 tests, all green, MempoolAuditorSpec included.

validatePool was a private method of the actor, closing over nodeSettings, the system clock and a hard-coded cost limit, so it could only be reached by starting an actor system. Move it to the companion object as a function of its arguments, with now and costLimit passed in, and return a named CleanupResult. The staleness filter is exposed separately as transactionsToValidate.

The actor keeps its behaviour: same messages, same reply, same log line.

Closes ergoplatform#1556
Covers the staleness filter and its boundary (exactly timeLimit old is not stale yet), still-valid transactions coming back with lastCost filled in, transactions spending non-existent inputs being reported as invalidated, a mixed pool splitting correctly, and the cost limit stopping the loop after exactly one transaction. No actor system involved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract logic from CleanupWorker actor and test it

1 participant