HathorNetwork / HathorNetwork/tx-mining-service

MinerTxJob.update_timestamp can stamp a tx with its own parent's timestamp, producing txs the fullnode rejects

Open
#172 1 comment 0 reactions 1 assignee Claimed by @tuliomir View on GitHub
Dominant language
Python
Stars
4
Forks
2
PR merge metrics
No merged PRs in 30d

Description

## Summary

When two transactions are submitted within the same wall-clock second and one is selected as the other's parent, the service stamps the child with the same timestamp as its parent and the fullnode deterministically rejects the mined result:

```
full validation failed: tx=263fb774… timestamp=1785951640, parent=051a87f8… timestamp=1785951640
```

The fullnode requires `tx.timestamp > parent.timestamp` (1-second granularity), but the service assigns parents and timestamp independently: parents are fetched as bare hashes, and the timestamp is always `int(time())`. The client cannot prevent or repair this — both fields are assigned inside the service, under the nonce — so its only recourse is to re-mine and resubmit.

This surfaced while running wallet-lib's integration suite with parallel jest workers (all txs flow through one tx-mining-service): a quiet DAG makes the newest tx almost always a tip, so any two same-second broadcasts can collide. The same mechanics apply to any two wallets sharing a service instance in the same second.

## Proposed fix

Make the stamped timestamp respect the parents:

```python
tx.timestamp = max(int(txstratum.time.time()), max_parent_timestamp + 1)
```

This stays within consensus rules: hathor-core accepts timestamps up to `MAX_FUTURE_TIMESTAMP_ALLOWED = 300` seconds in the future, and the clamp exceeds "now" by at most 1 second per same-second parent chain.

## Suggested rollout: DevMiner first

The `DevMiner` is the variant integration tests use, and it stamps the timestamp in exactly one place (`txstratum/dev/manager.py`, right before `solve_tx`). Landing the fix there first lets every client that runs integration tests against it validate the behavior thoroughly under real parallel load, at zero risk to production. Once proven, apply the same rule to the production path (`MinerTxJob.update_timestamp`), which also refreshes the timestamp every `JOB_UPDATE_INTERVAL = 2` seconds during mining.

The one design decision is how the service learns parent timestamps, since `backend.get_tx_parents()` returns hashes only:

1. Fetch each parent tx from the fullnode after resolving parents (two extra reads per job, cacheable) — single-repo change; or
2. Extend the fullnode's parents endpoint to return `(hash, timestamp)` pairs — cleaner, but cross-repo.

Option 1 seems sufficient to start, and the block path already has a TODO pointing the same direction (consuming timestamp bounds from the newer mining API).

---

## Annex — details for whoever picks this up

Everything below is reference material gathered while diagnosing; nothing here changes the proposal above.

**Code pointers (current `master`):**

- `txstratum/jobs.py` — `MinerTxJob.update_timestamp()`: `self._tx.timestamp = int(txstratum.time.time())`, unconditional. Called on job creation and refreshed by the protocol loop (`txstratum/protocol.py`, `JOB_UPDATE_INTERVAL = 2`).
- `txstratum/dev/manager.py` — DevMiner path: `tx.timestamp = int(txstratum.time.time())` immediately before `solve_tx`, single call site, already carries a comment about the fullnode's timestamp-delta validation.
- `txstratum/manager.py` — `add_parents()`: `parents: List[bytes] = await self.backend.get_tx_parents()` → `job.set_parents(parents)`. Hashes only; parent timestamps are never known to the service.
- `txstratum/api.py` — `MAX_TIMESTAMP_DELTA: int = 300` (service-side guard, mirrors the fullnode's tolerance).
- hathor-core: `MAX_FUTURE_TIMESTAMP_ALLOWED = 5 * 60` (`hathorlib/conf/settings.py`); enforced in `vertex_handler.py`. The `tx.timestamp > parent.timestamp` requirement is what produces the `full validation failed` above.

**Why a blind `now + 1` is not enough:** a same-second chain defeats it — tx A stamped `T+1` (parent at `T`) can itself be picked as parent by tx B submitted in the same second, which would also stamp `T+1`. The clamp must read the actual parent timestamps.

**Observed frequency** (wallet-lib integration suite on a privnet, ~800 broadcasts per full run, one shared tx-mining-service, `AVG_TIME_BETWEEN_BLOCKS: 1`):

- 2 parallel jest workers: 6 rejections per ~25 min run.
- 4 parallel jest workers: 73 rejections in ~14 min — enough to exhaust test-level retries and fail suites.
- Forensic pairing of every rejection confirmed the parent always came from a different OS process (another jest worker, or the integration-test-helper's own funding transaction — the helper is equally affected as a client).

**Client-side workaround in place:** wallet-lib's integration setup now re-mines and resubmits on this specific rejection (signatures cover only the funds part, so the same signed tx can be re-mined with fresh parents/timestamp). That unblocks parallel test execution but leaves every other client of the service exposed, which is why this issue proposes fixing it at the source.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.