network: XOR and IBLT trees advertise a transaction before the write transaction commits

[stevenvegt]

Summary

The XOR and IBLT trees are updated in memory inside the write transaction that stores a transaction, but they are served to peers without a database transaction. Between the tree update and the commit, the node advertises a transaction that its own GetTransaction cannot find. A peer that decodes the IBLT in that window asks for the transaction and gets nothing back.

Observed on the e2e test nuts-network/private-transactions in https://github.com/nuts-foundation/nuts-node/actions/runs/35200908093/job/105135215724. nodeB logged the warning for a reference it was still writing, and logged "Transaction created" for that same reference immediately after:

nodeB-1 | level=warning msg="Peer requested transaction we don't have" txRef=e624601e767fecabf3db5768469057ec229c9f97717fb0ae5bb4c5294a6f56e5
nodeB-1 | level=info msg="Transaction created" txRef=e624601e767fecabf3db5768469057ec229c9f97717fb0ae5bb4c5294a6f56e5

Mechanism

state.Add performs the whole write in one s.db.Write and ends with updateState, which calls ibltTree.write and xorTree.write (network/dag/state.go:169-220).

treeStore.write first mutates the in-memory tree and only then stages the dirty leaves into the write transaction (network/dag/treestore.go:62-67):

func (store *treeStore) write(tx stoabs.WriteTx, transaction Transaction) error {
	store.mutex.Lock()
	defer store.mutex.Unlock()

	store.tree.Insert(transaction.Ref(), transaction.Clock())
	return store.writeWithoutLock(tx)
}

state.XOR() and state.IBLT() read that in-memory tree directly, with no database transaction (network/dag/state.go:353-387). GetTransaction goes to the store, where readers see only the last committed snapshot. So from the moment updateState runs until the write transaction commits, the advertised state is ahead of what the node can serve.

That the in-memory trees are mutated before the commit is deliberate, and the OnRollback hook in state.Add exists to repair them when the commit fails ("Reloading the XOR and IBLT trees due to a DB transaction Rollback"). This issue is the other side of the same choice: during a successful commit the trees are correct but early.

How wide is the window

It is the remainder of the bbolt write transaction after updateState, including the commit and its fsync. The same transaction also writes the payload and two events, and runs under stoabs.WithWriteLock(). On a loaded node with a large database that is routinely tens of milliseconds and can spike higher.

Network latency does not protect against this. The window is local to the advertising node, and the peer only has to complete one round trip inside it. A nearby peer makes the round trip short; a slow disk makes the window long. Both get you there, so this is not limited to low-latency test setups.

Impact

By itself: a peer briefly requests a transaction that cannot be served yet. Combined with #4559, where a TransactionListQuery that resolves to nothing is never answered, the peer's sync with this node stalls for 30 seconds.

In steady state a node creates one transaction at a time, so the peer's query typically carries exactly one reference, which is the shape that triggers #4559. Batched sync requests are not affected, because at least one of the references resolves and a partial list is sent.

Suggested fix

The in-memory tree should not expose a transaction before the write transaction that stores it commits, while the persisted leaf data still has to be written inside that transaction for durability. Options worth weighing:

• compute the leaf data inside the write transaction but apply the in-memory Insert from an AfterCommit hook, which also removes the need for the OnRollback reload
• serve XOR() and IBLT() from a snapshot taken at the last commit
• gate XOR() and IBLT() on the write lock, which is the smallest change but puts protocol reads behind every write

The first two are preferable; the third trades the race for contention.

Note that this is about the transient window only. Persistent divergence between the trees and the DAG is a different problem, already handled by xorTreeRepair.