diff --git a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java index 8bf85082..3d0b651c 100644 --- a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java +++ b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java @@ -108,8 +108,9 @@ public class ElectrumServer { //Counts the rewinds of the header store, so that a proof can tell whether one happened while it was being obtained. Written under headerSyncLock static volatile int reorgCount; - //The deepest fork point the store has been rewound to this session, at or above which a stored height may have been proven against an orphaned - //header. Written only under headerSyncLock, which is what makes the min in reconcile atomic; volatile is for the readers that do not take it + //The deepest fork point the store has been rewound to this session, or that a wallet loaded in it was found to hold a proof from above, at or above + //which a stored height may have been proven against an orphaned header. Written only under headerSyncLock, which is what makes the min atomic; + //volatile is for the readers that do not take it static volatile int lastReorgForkHeight = Integer.MAX_VALUE; private static final Map walletSyncLocks = Collections.synchronizedMap(new HashMap<>()); @@ -260,7 +261,10 @@ public class ElectrumServer { reorgInvalidatedScriptHashes.clear(); proofWarnedPairs.clear(); proofsShownFalseWarnedPairs.clear(); - walletSyncLocks.values().forEach(syncLock -> syncLock.scriptHashesInitialized = false); + walletSyncLocks.values().forEach(syncLock -> { + syncLock.scriptHashesInitialized = false; + syncLock.storedProofsChecked = false; + }); } public void connect() throws ServerException { @@ -394,7 +398,10 @@ public class ElectrumServer { public static void clearRetrievedScriptHashes(Wallet wallet) { wallet.getNode(KeyPurpose.RECEIVE).getChildren().stream().map(ElectrumServer::getScriptHash).forEach(ElectrumServer::clearRetrievedScriptHash); wallet.getNode(KeyPurpose.CHANGE).getChildren().stream().map(ElectrumServer::getScriptHash).forEach(ElectrumServer::clearRetrievedScriptHash); - walletSyncLocks.computeIfAbsent(wallet.hashCode(), w -> new WalletSyncLock()).scriptHashesInitialized = false; + WalletSyncLock walletSyncLock = walletSyncLocks.computeIfAbsent(wallet.hashCode(), w -> new WalletSyncLock()); + walletSyncLock.scriptHashesInitialized = false; + walletSyncLock.storedProofsChecked = false; + walletSyncLock.storedProofsFromHeight = null; } private static void clearRetrievedScriptHash(String scriptHash) { @@ -485,6 +492,14 @@ public class ElectrumServer { if(isConnected()) { try { + if(!walletSyncLock.storedProofsChecked && isVerifyingTransactions()) { + walletSyncLock.storedProofsChecked = checkStoredProofs(wallet, walletSyncLock); + //Only a fetch of every node revisits what the comparison invalidated, and it is not made again once it has completed + if(nodes != null && hasReorgInvalidatedScriptHashes(wallet)) { + nodes = null; + } + } + //Taken before the fetch, so an invalidation arriving while this pass runs can be told from one the pass is acting on Set invalidatedBeforeFetch = Set.copyOf(reorgInvalidatedScriptHashes); Map previousScriptHashes = getCalculatedScriptHashes(wallet); @@ -558,6 +573,62 @@ public class ElectrumServer { } } + /** + * Compares the block each of the wallet's transactions was proven against with the header the store now holds at that height, once for a wallet + * as it is first fetched. A reorg reaches the wallets open when the store is rewound, and a wallet closed at the time - whether earlier this + * session or in one before it - still holds what the replaced block proved, at heights the server may well report unchanged. Where one is found, + * the wallet is treated as a reorg at that height would have treated it: its nodes above are fetched again and the stored block hashes compared. + *

+ * Only the transactions within the deepest reorg the store accepts of the block height the wallet stored are compared, which keeps an ordinary + * load from reading the store at all. That is every transaction a reorg could have reached where the store kept pace with the wallet, since the + * wallet heard of every reorg while it was open. It is not where the wallet's height advanced while nothing was being verified - on a Bitcoin + * Core connection, or with verification off - and the store stood still: a reorg reconciled later, below that window, is not looked for. + *

+ * Returns whether every one of them could be compared, which is false while the store has yet to reach one. The comparison is then made again on + * a later fetch, from the height it was first made from: the fetch in between moves the wallet's stored height on to the current tip. + */ + private static boolean checkStoredProofs(Wallet wallet, WalletSyncLock walletSyncLock) throws ServerException { + if(walletSyncLock.storedProofsFromHeight == null) { + int startHeight = Network.get().getHeaderCheckpoints().getMaxHeight() + 1; + Integer storedHeight = wallet.getStoredBlockHeight(); + walletSyncLock.storedProofsFromHeight = storedHeight == null ? startHeight : Math.max(startHeight, storedHeight - MAX_REORG_DEPTH + 1); + } + + int fromHeight = walletSyncLock.storedProofsFromHeight; + List proven = wallet.getTransactions().values().stream().filter(blkTx -> blkTx.getBlockHash() != null && blkTx.getHeight() >= fromHeight).toList(); + if(proven.isEmpty()) { + return true; //the ordinary case, in which the store is not read, nor loaded for a wallet that has no use for it + } + + try { + //Read without the header sync lock, which a catch up holds across its fetches. A header the sync is about to replace compares as held, + //and the reorg that replaces it then reaches this wallet as it does any other that is open + HeaderStore store = getHeaderStore(); + int orphanedHeight = Integer.MAX_VALUE; + boolean compared = true; + for(BlockTransaction blkTx : proven) { + Sha256Hash storedHash = store.getHash(blkTx.getHeight()); + if(storedHash == null) { + compared = false; + } else if(!storedHash.equals(blkTx.getBlockHash())) { + orphanedHeight = Math.min(orphanedHeight, blkTx.getHeight()); + } + } + + if(orphanedHeight < Integer.MAX_VALUE) { + int forkHeight = orphanedHeight - 1; + synchronized(headerSyncLock) { + lastReorgForkHeight = Math.min(lastReorgForkHeight, forkHeight); + } + invalidateWalletScriptHashesForReorg(wallet, forkHeight); + } + + return compared; + } catch(IOException e) { + throw new ServerException("Could not read the block header store", e); + } + } + public Map> getHistory(Wallet wallet) throws ServerException { Map> receiveTransactionMap = new TreeMap<>(); getHistory(wallet, KeyPurpose.RECEIVE, receiveTransactionMap); @@ -3542,6 +3613,8 @@ public class ElectrumServer { private static class WalletSyncLock { public boolean scriptHashesInitialized; + public boolean storedProofsChecked; + public Integer storedProofsFromHeight; } public static class TransactionHistoryService extends Service { diff --git a/src/test/java/com/sparrowwallet/sparrow/net/TransactionProofTest.java b/src/test/java/com/sparrowwallet/sparrow/net/TransactionProofTest.java index ef2eac1a..059d72c6 100644 --- a/src/test/java/com/sparrowwallet/sparrow/net/TransactionProofTest.java +++ b/src/test/java/com/sparrowwallet/sparrow/net/TransactionProofTest.java @@ -1020,6 +1020,113 @@ public class TransactionProofTest { assertFalse(ElectrumServer.reorgInvalidatedScriptHashes.contains(scriptHash), "and the exemption lasts for exactly that one fetch"); } + /** + * A reorg reaches the wallets that are open when the store is rewound. One opened afterwards, in the same session or a later one, still holds what + * the replaced block proved at a height the server reports unchanged, so its node is never fetched and the stored height never looked at. The + * block recorded with the transaction is compared with the store as the wallet is first fetched, and the height stands only if it is proven again. + */ + @Test + public void reprovesAStoredHeightWhoseBlockTheStoreNoLongerHolds() throws Exception { + Wallet wallet = testWallet(); + WalletNode node = wallet.getNode(KeyPurpose.RECEIVE).getChildren().iterator().next(); + Transaction transaction = confirmedPayment(wallet, node); + wallet.updateTransactions(Map.of(transaction.getTxId(), new BlockTransaction(transaction.getTxId(), PROVEN_HEIGHT, null, 0L, transaction, Sha256Hash.ZERO_HASH))); + //The deepest a reorg since could have reached: one that forked a hundred blocks below a tip at the height the wallet last stored + wallet.setStoredBlockHeight(PROVEN_HEIGHT + 99); + + new ElectrumServer().fetchAndCalculateHistory(wallet, null, null); + + //The payment is not in the block the store holds at that height, so the proof served for it does not reconstruct + assertTrue(server.getProofRequests() > 0); + assertEquals(0, wallet.getWalletTransaction(transaction.getTxId()).getHeight()); + assertEquals(0, node.getTransactionOutputs().iterator().next().getHeight()); + assertFalse(ElectrumServer.reorgInvalidatedScriptHashes.contains(ElectrumServer.getScriptHash(node)), "the fetch that acted on the invalidation clears it"); + } + + /** + * The comparison is made by whichever fetch comes first once transactions are being verified, which need not be one of every node: the store + * may not have reached a stored proof on the first, or the server may only then have caught up to the last pin. A fetch of other nodes is + * widened to all of them, since the comparison is not made again and nothing else would revisit the node it invalidated. + */ + @Test + public void reprovesAStoredHeightFromAFetchOfOtherNodes() throws Exception { + Wallet wallet = testWallet(); + List nodes = new ArrayList<>(wallet.getNode(KeyPurpose.RECEIVE).getChildren()); + Transaction transaction = confirmedPayment(wallet, nodes.get(0)); + wallet.updateTransactions(Map.of(transaction.getTxId(), new BlockTransaction(transaction.getTxId(), PROVEN_HEIGHT, null, 0L, transaction, Sha256Hash.ZERO_HASH))); + + new ElectrumServer().fetchAndCalculateHistory(wallet, null, Set.of(nodes.get(1))); + + assertTrue(server.getProofRequests() > 0); + assertEquals(0, wallet.getWalletTransaction(transaction.getTxId()).getHeight()); + assertFalse(ElectrumServer.reorgInvalidatedScriptHashes.contains(ElectrumServer.getScriptHash(nodes.get(0)))); + } + + /** + * A store that has not reached a stored proof cannot be compared with it, so the comparison waits for a later fetch. The fetch in between moves + * the wallet's stored height on to the current tip, which for a wallet closed long enough is far above the transaction: the comparison is made + * from the height it was first made from, or the wait would end with nothing compared. + */ + @Test + public void comparesAStoredHeightTheStoreReachesOnALaterFetch() throws Exception { + Wallet wallet = testWallet(); + WalletNode node = wallet.getNode(KeyPurpose.RECEIVE).getChildren().iterator().next(); + Transaction transaction = confirmedPayment(wallet, node); + wallet.updateTransactions(Map.of(transaction.getTxId(), new BlockTransaction(transaction.getTxId(), PROVEN_HEIGHT, null, 0L, transaction, Sha256Hash.ZERO_HASH))); + wallet.setStoredBlockHeight(PROVEN_HEIGHT); + HeaderStore store = ElectrumServer.getHeaderStore(); + store.truncate(PROVEN_HEIGHT - 1); + + new ElectrumServer().fetchAndCalculateHistory(wallet, null, null); + assertEquals(0, server.getProofRequests()); + assertEquals(PROVEN_HEIGHT, wallet.getWalletTransaction(transaction.getTxId()).getHeight()); + + //What a completed fetch does to the stored height, and the header sync to the store, before the next one + wallet.setStoredBlockHeight(PROVEN_HEIGHT + 200); + store.append(chain.subList(PROVEN_HEIGHT - 1, CHAIN_LENGTH - 1)); + + new ElectrumServer().fetchAndCalculateHistory(wallet, null, null); + assertTrue(server.getProofRequests() > 0); + assertEquals(0, wallet.getWalletTransaction(transaction.getTxId()).getHeight()); + } + + /** + * The path that must keep working: a wallet whose stored proofs are of blocks the store still holds is fetched as it always was, with nothing + * proven again and no node fetched that the server reports unchanged. + */ + @Test + public void doesNotReproveAStoredHeightWhoseBlockTheStoreHolds() throws Exception { + Wallet wallet = testWallet(); + WalletNode node = wallet.getNode(KeyPurpose.RECEIVE).getChildren().iterator().next(); + Transaction transaction = confirmedPayment(wallet, node); + + new ElectrumServer().fetchAndCalculateHistory(wallet, null, null); + + assertEquals(0, server.getProofRequests()); + assertEquals(Integer.MAX_VALUE, ElectrumServer.lastReorgForkHeight); + assertEquals(PROVEN_HEIGHT, wallet.getWalletTransaction(transaction.getTxId()).getHeight()); + assertEquals(PROVEN_HEIGHT, node.getTransactionOutputs().iterator().next().getHeight()); + } + + /** + * A wallet was told of every reorg up to the height it stored, and none since can have rewound the store by more than the depth it accepts. A + * transaction further below the stored height than that is not compared, which is what leaves an ordinary wallet load reading nothing. + */ + @Test + public void doesNotCompareAStoredHeightNoReorgSinceCouldHaveReached() throws Exception { + Wallet wallet = testWallet(); + WalletNode node = wallet.getNode(KeyPurpose.RECEIVE).getChildren().iterator().next(); + Transaction transaction = confirmedPayment(wallet, node); + wallet.updateTransactions(Map.of(transaction.getTxId(), new BlockTransaction(transaction.getTxId(), PROVEN_HEIGHT, null, 0L, transaction, Sha256Hash.ZERO_HASH))); + wallet.setStoredBlockHeight(PROVEN_HEIGHT + 100); + + new ElectrumServer().fetchAndCalculateHistory(wallet, null, null); + + assertEquals(0, server.getProofRequests()); + assertEquals(Integer.MAX_VALUE, ElectrumServer.lastReorgForkHeight); + assertEquals(PROVEN_HEIGHT, wallet.getWalletTransaction(transaction.getTxId()).getHeight()); + } + /** * A payment to the node, stored as confirmed and reported so by the server, as a wallet is when a reorg reaches it. */