diff --git a/include/balance_sheet.h b/include/balance_sheet.h index 0b5b5be..5ad4b0d 100644 --- a/include/balance_sheet.h +++ b/include/balance_sheet.h @@ -16,6 +16,20 @@ typedef struct { uint8_t address[32]; // For now just the SHA-256 of the public key; allows representation in different encodings (base58, bech32, etc) without changing the underlying data structure uint256_t balance; + /** + * Timestamp (unix ms) of the most recent transaction this address SENT that is in the chain. + * + * Replay protection. Without it any historical transaction could be rebroadcast and mined a + * second time, debiting the sender again -- with UTXOs the spent inputs make that impossible, + * but an account model has nothing to stop it. A non-coinbase transaction is only valid if its + * timestamp is strictly greater than this, so a byte-identical replay (same timestamp, same + * hash) can never be included twice. Enforced in Chain_AddBlockLocked; see the note there. + * + * Rebuilt for free by the rollback's balance-sheet replay, so a reorg cannot leave it stale. + * Persisted with the rest of the entry -- note the file has no height marker, so a balance + * sheet that is out of sync with the chain silently resets this to 0 for every account. + **/ + uint64_t lastTxTimestamp; // TODO: Additional things } balance_sheet_entry_t; diff --git a/include/block/chain.h b/include/block/chain.h index 20f2f2e..732b3e3 100644 --- a/include/block/chain.h +++ b/include/block/chain.h @@ -90,6 +90,20 @@ bool Chain_IsInitialBlockDownload(blockchain_t* chain); // want to report it. uint64_t Chain_ReorgPenaltyForDepth(uint64_t reorgDepth); +/** + * Replay guard: true if every non-coinbase transaction in `block` is newer than its own sender's + * last included transaction, and newer than that same sender's earlier transactions in this block. + * + * Reads the balance sheet's per-account `lastTxTimestamp` (see balance_sheet.h). Senders are + * considered independently -- one account's transactions say nothing about another's ordering, so + * an ordinary block full of different senders always passes. Coinbase is exempt. + * + * Exposed rather than inlined so this can be tested directly; Chain_AddBlockLocked calls it as part + * of block validation, which is what makes it apply to mining, sync, broadcast, orphan attach and + * reorg alike. +**/ +bool Chain_BlockRespectsSenderOrdering(const block_t* block); + // Recompute `currentSupply` and `currentReward` from the in-memory chain blocks. // Returns true on success and updates runtime state globals. bool Chain_RecomputeRuntimeState(blockchain_t* chain); diff --git a/include/block/transaction.h b/include/block/transaction.h index 7ebeeac..b36165d 100644 --- a/include/block/transaction.h +++ b/include/block/transaction.h @@ -23,7 +23,11 @@ static inline bool Address_IsCoinbase(const uint8_t address[32]) { // 168 bytes total for v1 #pragma pack(push, 1) // Ensure no padding for consistent file storage typedef struct { - uint64_t timestamp; // Unix timestamp in seconds - not enforced, but used for uniqueness when everything else is the same. + uint64_t timestamp; // Unix timestamp in MILLISECONDS (get_current_time_ms). Two of a sender's + // transactions must have strictly increasing timestamps -- see + // lastTxTimestamp in balance_sheet.h. Millisecond resolution is what makes + // an exact collision mean 'byte-identical replay' rather than 'two real + // transactions that happened to coincide'. uint64_t fee; // Rewarded to the miner; can be zero, but the miner may choose to ignore transactions with very low fees uint64_t amount1; uint64_t amount2; diff --git a/include/constants.h b/include/constants.h index 018078a..b345d5e 100644 --- a/include/constants.h +++ b/include/constants.h @@ -91,6 +91,25 @@ static const uint64_t REORG_PENALTY_REF_BLOCK_TIME = 150ULL; // reference block // from overflow rather than to bound the penalty in any meaningful sense. static const uint64_t REORG_PENALTY_MAX_DEPTH = 1000ULL; +/** + * Mempool transaction timestamp policy. LOCAL POLICY, NOT CONSENSUS. + * + * These govern what this node is willing to hold and relay; a block containing a transaction that + * violates either is still accepted. That separation is deliberate -- a node with a skewed clock + * must not be able to fork itself off the network over an admission rule. + * + * A too-OLD timestamp needs no rule here: the per-account replay guard (see balance_sheet.h) already + * refuses anything at or below a sender's last included transaction. +**/ +// Refuse to admit a transaction dated further ahead than this of OUR OWN CLOCK. Measured against +// the clock and not against the chain tip on purpose: on a quiet chain the tip can be hours old, and +// judging "future" against it would refuse honest transactions exactly when blocks are sparse. +static const uint64_t TX_MAX_FUTURE_DRIFT_MS = 2ULL * 60ULL * 60ULL * 1000ULL; // 2 hours +// Drop transactions older than this from the mempool, so it is not inflated by junk that will never +// be mined. Roughly the ~4 days DIFFICULTY_ADJUSTMENT_INTERVAL spans, but expressed in milliseconds +// so it does not drift if the block time changes. +static const uint64_t TX_EXPIRY_MS = 4ULL * 24ULL * 60ULL * 60ULL * 1000ULL; // 4 days + // Upper bound on pooled orphan blocks. Orphans are accepted before the chain-derived difficulty // check (that lives in Chain_AddBlock, which orphans only reach on attach), so without a cap a // peer can push blocks at an arbitrary height until the node runs out of memory. diff --git a/include/txmempool.h b/include/txmempool.h index 18071ce..2e003c0 100644 --- a/include/txmempool.h +++ b/include/txmempool.h @@ -17,6 +17,25 @@ bool TxMempool_Snapshot(signed_transaction_t** outTxs, size_t* outCount); void TxMempool_Print(); // Remove a transaction from the mempool by its hash. Returns true if removed. bool TxMempool_Remove(const uint8_t* txHash); + +/** + * Admission policy: should this transaction be held and relayed? + * + * LOCAL POLICY, NOT CONSENSUS. A block containing a transaction this rejects is still accepted -- + * see TX_MAX_FUTURE_DRIFT_MS / TX_EXPIRY_MS in constants.h for why the two are kept apart. + * + * Both bounds are measured against the node's own clock, NOT against the chain tip's timestamp. + * Measuring "future" against the last block assumes blocks keep arriving: on a quiet chain the tip + * can be hours old, and an honest transaction created right now would look hours ahead of it and be + * refused. Sending would become impossible exactly when the chain is idle. + * + * Deliberately NOT applied when a rollback returns transactions to the pool: those were already in + * the chain, so they are legitimate by definition and must not be dropped for looking old. +**/ +bool TxMempool_PolicyAccepts(const signed_transaction_t* tx, uint64_t nowMs); + +// Drop transactions older than TX_EXPIRY_MS. Returns how many were removed. +size_t TxMempool_PruneExpired(uint64_t nowMs); void TxMempool_Destroy(); #endif diff --git a/src/block/chain.c b/src/block/chain.c index 7c32e34..ab6898c 100644 --- a/src/block/chain.c +++ b/src/block/chain.c @@ -83,7 +83,15 @@ static bool CreditAddress(const uint8_t address[32], uint64_t amount) { return BalanceSheet_Insert(entry) >= 0; } -static bool DebitAddress(const uint8_t address[32], const uint256_t* amount) { +/** + * Debit a sender and advance its replay guard in one step. + * + * `txTimestamp` is the timestamp of the transaction doing the spending; it becomes the account's + * `lastTxTimestamp`, which is what makes a byte-identical replay of that transaction invalid later. + * Taking it here rather than updating the entry separately means the debit and the guard cannot + * drift apart -- every path that spends from an account necessarily advances it. +**/ +static bool DebitAddress(const uint8_t address[32], const uint256_t* amount, uint64_t txTimestamp) { if (!address || !amount) { return false; } @@ -101,6 +109,10 @@ static bool DebitAddress(const uint8_t address[32], const uint256_t* amount) { return false; } + if (txTimestamp > entry.lastTxTimestamp) { + entry.lastTxTimestamp = txTimestamp; + } + return BalanceSheet_Insert(entry) >= 0; } @@ -424,6 +436,31 @@ static bool Chain_AddBlockLocked(blockchain_t* chain, block_t* block) { free(spendableTxs); + /** + * Replay guard: each sender's transaction timestamps must strictly increase. + * + * Without this, any transaction already in the chain could be rebroadcast and mined again, + * debiting the sender a second time. UTXO chains get this for free because the inputs no + * longer exist; an account model has nothing to stop it. Timestamps are unix milliseconds, + * so two genuinely distinct transactions never collide, and an exact collision means a + * byte-identical copy -- precisely what must be refused. + * + * Checked HERE, with the rest of block validation and before the push, rather than in the + * ledger pass below: that pass runs after the block is already in the chain and can only + * return false, leaving an invalid block behind. + * + * Placement in Chain_AddBlockLocked is what makes it apply everywhere -- mining, broadcast, + * windowed sync, orphan attach and reorg apply all funnel through here, and it is the only + * insertion point into the chain besides the header-only disk load. Do not add a second + * copy on the receive path where the symptom happens to be visible. + **/ + if (!Chain_BlockRespectsSenderOrdering(block)) { + printf("Chain_AddBlock: validation failed: blockIndex=%zu contains a transaction that is " + "not newer than its sender's last -- replay or out-of-order\n", expectedIndex); + ok = false; + break; + } + // Push the block only after validation succeeds. block_t* blk = (block_t*)DynArr_push_back(chain->blocks, block); if (!blk) { ok = false; break; } @@ -462,7 +499,8 @@ static bool Chain_AddBlockLocked(blockchain_t* chain, block_t* block) { if (!Address_IsCoinbase(tx->transaction.senderAddress)) { uint256_t spend; - if (!BuildSpendAmount(tx, &spend) || !DebitAddress(tx->transaction.senderAddress, &spend)) { + if (!BuildSpendAmount(tx, &spend) || + !DebitAddress(tx->transaction.senderAddress, &spend, tx->transaction.timestamp)) { fprintf(stderr, "Error: Failed to debit sender balance during block addition. Bailing!\n"); ok = false; break; } @@ -631,6 +669,52 @@ static bool Chain_RollbackToHeightLocked(blockchain_t* chain, size_t height) { return true; // nothing to do } + /** + * Return the discarded blocks' transactions to the mempool before anything is freed. + * + * Without this a reorg silently destroys them: they were removed from the mempool when mined + * (see Chain_AddBlockLocked) and nothing ever put them back, so a transaction that existed only + * in an orphaned block was gone from both the chain and the pool and would never be mined + * unless its sender happened to rebroadcast. An accidental orphan should cost a transaction one + * block of delay, not its existence. + * + * Coinbases are deliberately NOT restored -- they are bound to a specific height and reward and + * are invalid anywhere else. + * + * A transaction that also appears in the branch replacing this one needs no special handling: + * Chain_ReplaceBranch rolls back and then applies under a single lock acquisition, and + * Chain_AddBlockLocked removes every applied transaction from the pool again. So it is + * re-inserted here and removed again moments later, with no window in which a miner could pick + * up a copy of something already back in the chain. That ordering is what makes this safe in an + * account model, where re-mining a transaction would debit the sender twice. + * + * Blocks are usually header-only by now (anything saved or loaded has transactions == NULL), so + * this goes through the same borrow/disk-fallback pair the balance-sheet replay below uses. + **/ + for (size_t i = cur; i > height; --i) { + const size_t idx = i - 1; + + block_t* source = NULL; + bool loadedFromDisk = false; + if (!Chain_BorrowBlockTransactions(chain, idx, &source, &loadedFromDisk)) { + // Nothing recoverable for this block; the rollback itself must still proceed. + continue; + } + + if (source && source->transactions) { + const size_t txCount = DynArr_size(source->transactions); + for (size_t t = 0; t < txCount; ++t) { + signed_transaction_t* tx = (signed_transaction_t*)DynArr_at(source->transactions, t); + if (!tx || Address_IsCoinbase(tx->transaction.senderAddress)) { + continue; + } + (void)TxMempool_Insert(*tx); + } + } + + Chain_ReturnBlockTransactions(source, loadedFromDisk); + } + // Remove blocks above height for (size_t i = cur; i > height; --i) { size_t idx = i - 1; @@ -702,6 +786,12 @@ static bool Chain_RollbackToHeightLocked(blockchain_t* chain, size_t height) { senderEntry.balance = uint256_from_u64(0); } (void)uint256_subtract(&senderEntry.balance, &spend); + // Rebuild the replay guard in the same pass that rebuilds the balance. Because this + // replay is what a rollback already does, the guard needs no invalidation logic of + // its own -- after any reorg it reflects exactly the blocks that remain. + if (tx->transaction.timestamp > senderEntry.lastTxTimestamp) { + senderEntry.lastTxTimestamp = tx->transaction.timestamp; + } (void)BalanceSheet_Insert(senderEntry); // Credit recipient1 @@ -872,6 +962,58 @@ static void Chain_FreeBlockArray(block_t** blocks, size_t count) { free(blocks); } +bool Chain_BlockRespectsSenderOrdering(const block_t* block) { + if (!block || !block->transactions) { + return false; + } + + const size_t txCount = DynArr_size(block->transactions); + + for (size_t i = 0; i < txCount; ++i) { + const signed_transaction_t* tx = (const signed_transaction_t*)DynArr_at(block->transactions, i); + if (!tx || Address_IsCoinbase(tx->transaction.senderAddress)) { + continue; // coinbase is exempt -- see balance_sheet.h + } + + // Baseline is THIS sender's own last included transaction. + uint64_t lastSeen = 0; + balance_sheet_entry_t senderEntry; + if (BalanceSheet_Lookup((uint8_t*)tx->transaction.senderAddress, &senderEntry)) { + lastSeen = senderEntry.lastTxTimestamp; + } + + /** + * Fold in earlier transactions from THE SAME SENDER in this same block, which the sheet + * does not know about yet: the ledger applies a block in order, so they must be ordered. + * + * The sender comparison is load-bearing. Without it this would absorb every other sender's + * timestamps and reject ordinary multi-sender blocks -- Alice's transaction would be judged + * against Bob's. Senders are strictly independent here. + * + * Quadratic in the worst case (a whole block from one sender). Fine while blocks are small; + * worth a per-sender map if they grow. + **/ + for (size_t j = 0; j < i; ++j) { + const signed_transaction_t* prev = (const signed_transaction_t*)DynArr_at(block->transactions, j); + if (!prev || Address_IsCoinbase(prev->transaction.senderAddress)) { + continue; + } + if (memcmp(prev->transaction.senderAddress, tx->transaction.senderAddress, 32) != 0) { + continue; // different sender -- says nothing about this one's ordering + } + if (prev->transaction.timestamp > lastSeen) { + lastSeen = prev->transaction.timestamp; + } + } + + if (tx->transaction.timestamp <= lastSeen) { + return false; + } + } + + return true; +} + uint64_t Chain_ReorgPenaltyForDepth(uint64_t reorgDepth) { return FetchScheduler_ComputeReorgPenaltyBlocks(reorgDepth); } diff --git a/src/main.c b/src/main.c index e7531c9..a70bc7c 100644 --- a/src/main.c +++ b/src/main.c @@ -202,6 +202,52 @@ static void AddCoinbaseTransaction(block_t* block, const uint8_t minerAddress[32 Block_AddTransaction(block, &coinbaseTx); } +/** + * Put each sender's transactions into timestamp order, without disturbing anyone else's position. + * + * Chain_AddBlockLocked requires a sender's timestamps to strictly increase and walks a block in + * order, so a sender's later, higher-fee transaction sorted ahead of their earlier one would make + * the block we just built invalid by our own rule. + * + * This cannot be folded into CompareTransactionPriority. "Higher fee first, except same sender goes + * by time" is not a strict weak ordering -- A beats B on fee, B beats C on fee, C beats A on time -- + * and qsort with an inconsistent comparator is undefined behaviour. So sort by priority first, then + * permute each sender's transactions among the slots they already occupy: fee-based slot allocation + * survives untouched and only the order within one sender's own slots changes. + * + * Selection sort restricted to each sender's slots. Quadratic in the worst case (a whole block from + * one sender); fine while blocks are small. +**/ +static void OrderSameSenderByTimestamp(signed_transaction_t* txs, size_t count) { + if (!txs || count < 2) { + return; + } + + for (size_t i = 0; i < count; ++i) { + if (Address_IsCoinbase(txs[i].transaction.senderAddress)) { + continue; + } + + size_t earliest = i; + for (size_t j = i + 1; j < count; ++j) { + // Only ever compare against the SAME sender, so other senders keep their fee ranking. + if (memcmp(txs[j].transaction.senderAddress, txs[i].transaction.senderAddress, 32) != 0) { + continue; + } + if (txs[j].transaction.timestamp < txs[earliest].transaction.timestamp) { + earliest = j; + } + } + + if (earliest != i) { + // Both slots belong to the same sender, so this swap cannot reorder anyone else. + signed_transaction_t tmp = txs[i]; + txs[i] = txs[earliest]; + txs[earliest] = tmp; + } + } +} + static int CompareTransactionPriority(const void* lhs, const void* rhs) { const signed_transaction_t* left = (const signed_transaction_t*)lhs; const signed_transaction_t* right = (const signed_transaction_t*)rhs; @@ -241,6 +287,7 @@ static bool BuildSpendableMempoolSelection( if (snapshot && snapshotCount > 1) { qsort(snapshot, snapshotCount, sizeof(signed_transaction_t), CompareTransactionPriority); + OrderSameSenderByTimestamp(snapshot, snapshotCount); } signed_transaction_t* acceptedTxs = NULL; @@ -1162,7 +1209,12 @@ int main(int argc, char* argv[]) { printf("send committed in mined block\n"); */ - // Insert into txmempool + // Insert into txmempool, subject to the same admission policy peers apply, so we do + // not broadcast something the rest of the network will decline to hold. + if (!TxMempool_PolicyAccepts(&spendTx, get_current_time_ms())) { + printf("transaction timestamp is outside the accepted window, not sending\n"); + continue; + } if (TxMempool_Insert(spendTx) < 0) { printf("failed to add transaction to mempool, transaction rejected\n"); continue; diff --git a/src/nets/net_node.c b/src/nets/net_node.c index a102cde..34b576b 100644 --- a/src/nets/net_node.c +++ b/src/nets/net_node.c @@ -457,6 +457,14 @@ static void* Node_MaintenanceThread(void* arg) { BalanceSheet_SaveToFile(chainDataDir); } } + // Drop transactions too old to be worth holding, so the pool is not inflated by junk that + // will never be mined. Policy only -- a block containing one is still accepted. + { + const size_t pruned = TxMempool_PruneExpired(get_current_time_ms()); + if (pruned > 0) { + printf("Maintenance: pruned %zu expired transaction(s) from the mempool\n", pruned); + } + } // Reclaim outbound slots whose peer has disconnected so they can be reused. Node_ReapDeadOutbound(n); // Peer discovery tick: ping/query connected peers and connect to the best-ping discoveries. @@ -1235,7 +1243,14 @@ void Node_Server_OnData(tcp_connection_t* client) { return; } - // Push to mempool if it's not already present + // Push to mempool if it's not already present, subject to admission policy. + // Policy only: a block containing this transaction is still accepted even if we + // decline to hold or relay it ourselves. + if (!TxMempool_PolicyAccepts(&tx, get_current_time_ms())) { + printf("Declined transaction from node %u: timestamp outside the accepted window\n", + client ? client->connectionId : 0U); + return; + } if (!TxMempool_Lookup(txHash, &tx)) { if (TxMempool_Insert(tx) >= 0) { printf("Added transaction %s from node %u to mempool\n", txHashHex, client ? client->connectionId : 0U); diff --git a/src/txmempool.c b/src/txmempool.c index 6ac4bda..95dd105 100644 --- a/src/txmempool.c +++ b/src/txmempool.c @@ -1,4 +1,5 @@ #include +#include #include static pthread_mutex_t g_txMempoolLock; @@ -12,6 +13,52 @@ void TxMempool_Init() { g_txMempoolLockInitialized = true; } +bool TxMempool_PolicyAccepts(const signed_transaction_t* tx, uint64_t nowMs) { + if (!tx) { + return false; + } + + const uint64_t ts = tx->transaction.timestamp; + + // Dated too far in the future, measured against OUR CLOCK rather than the chain tip -- see the + // note in the header. Refusing this also limits the one real footgun in the replay guard: a + // wildly future timestamp permanently advances that account's lastTxTimestamp and locks it out + // until real time catches up. + if (ts > nowMs && (ts - nowMs) > TX_MAX_FUTURE_DRIFT_MS) { + return false; + } + + // Too old to be worth holding. Not a validity judgement -- just pool hygiene. + if (nowMs > ts && (nowMs - ts) > TX_EXPIRY_MS) { + return false; + } + + return true; +} + +size_t TxMempool_PruneExpired(uint64_t nowMs) { + if (!txMempool) { + return 0; + } + + size_t removed = 0; + + pthread_mutex_lock(&g_txMempoolLock); + for (khiter_t k = kh_begin(txMempool); k != kh_end(txMempool); ++k) { + if (!kh_exist(txMempool, k)) { + continue; + } + const uint64_t ts = kh_value(txMempool, k).transaction.timestamp; + if (nowMs > ts && (nowMs - ts) > TX_EXPIRY_MS) { + kh_del(tx_mempool_map_m, txMempool, k); + removed++; + } + } + pthread_mutex_unlock(&g_txMempoolLock); + + return removed; +} + int TxMempool_Insert(signed_transaction_t tx) { if (!txMempool) { return -1; }