diff --git a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java index 7a9eea28..fbf8149f 100644 --- a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java +++ b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java @@ -1663,6 +1663,22 @@ public class ElectrumServer { } } + /** + * The tip of the verified header store, whose header is null while the store holds nothing above the last pin. + */ + static ChainTip getStoreTip() throws ServerException { + HeaderStore store = getHeaderStore(); + try { + //The store's monitor, so that a reorg cannot truncate it between reading the tip height and the header at that height + synchronized(store) { + int tipHeight = store.getTipHeight(); + return new ChainTip(tipHeight, store.getHeader(tipHeight)); + } + } catch(IOException e) { + throw new ServerException("Could not read the block header store", e); + } + } + /** * The header at the given height verified against the compiled-in checkpoints, or null where the connected server cannot substantiate it, which is * reported as a refusal. Heights above the last pin are served from the store, and those below it by hash linkage to a pin. @@ -3407,17 +3423,57 @@ public class ElectrumServer { //The pair from the event that last restarted this service: the height and the header of one announcement, never of two private volatile ChainTip announcedTip; + //The announcement whose run last failed, so that one still unsubstantiated when its retry fails too is refused rather than retried indefinitely. + //Held as the tip rather than a count, since a run cancelled by a later announcement can still fail after it and would spend that one's retry + private volatile ChainTip failedTip; + @Override protected Task createTask() { return new Task<>() { @Override protected Void call() throws Exception { - syncAnnouncedHeaders(announcedTip); + ChainTip tip = announcedTip; + try { + syncAnnouncedHeaders(tip); + } catch(VerificationException | UnsupportedMethodException e) { + refuseAnnouncedTip(tip, e); + } catch(ServerException | ElectrumServerRpcException e) { + //A failed call says nothing about the chain on its own, but a server answering every request for these headers with an error + //has not substantiated the tip any more than one serving the wrong headers + if(failedTip != tip || !isConnected()) { + failedTip = tip; + throw e; + } + + refuseAnnouncedTip(tip, e); + } + return null; } }; } + /** + * Warns of an announced tip the header sync could not substantiate, and sets the chain tip back to the verified store tip where the store holds a + * header to set it back to, an empty store leaving only the warning. An announced header need + * only meet the target it claims for itself, which at the minimum difficulty costs nothing to produce, so until it links into the chain the height + * it arrived with is only the server's claim, as is every confirmation count taken from it. + */ + private void refuseAnnouncedTip(ChainTip tip, Exception e) throws ServerException { + ChainTip storeTip = getStoreTip(); + Platform.runLater(() -> { + //A run overtaken by a later announcement has nothing left to correct + if(tip != announcedTip) { + return; + } + + if(storeTip.header() != null) { + EventManager.get().post(new NewBlockEvent(storeTip.height(), storeTip.header())); + } + warnInvalidTip("Could not verify the block header chain to the tip announced at height " + tip.height() + ": " + e.getMessage()); + }); + } + /** * The body of a run, which happens on a background thread: the connection check is therefore the transport level one, since AppServices reads * a JavaFX Service and may only be called from the application thread. Without the check a retry firing after the connection closed would @@ -3434,8 +3490,9 @@ public class ElectrumServer { } catch(UnsupportedMethodException e) { //Without this call the store can never advance, so verification would refuse every new confirmation for the rest of the session if(isVerificationMandatory()) { - //Leaving the capability on is what lets the next wallet history thread raise this and rotate the server, which this service cannot do - log.warn("Server does not support " + e.getMethod() + ", which is required to verify transactions"); + //Leaving the capability on is what lets the next wallet history thread raise this and rotate the server, which this service cannot do. + //Rethrown so the tip is refused: where verification is mandatory, a server without the call has not substantiated the height it announced + throw e; } else { log.warn("Server does not support " + e.getMethod() + ", disabling transaction verification for this session"); serverCapability.withMerkleProofs(false); diff --git a/src/test/java/com/sparrowwallet/sparrow/net/HeaderSyncTest.java b/src/test/java/com/sparrowwallet/sparrow/net/HeaderSyncTest.java index e6f178cc..27dcff9c 100644 --- a/src/test/java/com/sparrowwallet/sparrow/net/HeaderSyncTest.java +++ b/src/test/java/com/sparrowwallet/sparrow/net/HeaderSyncTest.java @@ -13,6 +13,7 @@ import com.sparrowwallet.sparrow.ChainTip; import com.sparrowwallet.sparrow.EventManager; import com.sparrowwallet.sparrow.SparrowWallet; import com.sparrowwallet.sparrow.event.ChainReorgEvent; +import com.sparrowwallet.sparrow.io.Config; import com.sparrowwallet.sparrow.io.Storage; import com.google.common.eventbus.Subscribe; import org.junit.jupiter.api.AfterAll; @@ -38,6 +39,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -741,6 +743,55 @@ public class HeaderSyncTest { assertEquals(0, fake.getChunkRequests()); } + /** + * An announced header need only meet the target it claims for itself, which at the minimum difficulty costs nothing to produce, so a server can + * announce a height the chain has not reached. The sync refuses such a tip, and the store tip it leaves behind - holding the honest headers the + * server did serve - is the verified height the chain tip is set back to. An empty store has no header to set it back to. + */ + @Test + public void refusesAnAnnouncedTipAboveTheChainTheServerServes() throws Exception { + List chain = mineChain(Network.REGTEST.getGenesisHeader(), 12, CHAIN_TIME); + BlockHeader unlinked = mineChain(Network.REGTEST.getGenesisHeader(), 1, BRANCH_TIME).getFirst(); + assertNull(ElectrumServer.getStoreTip().header()); + + seedStore(chain, 10); + serve(chain); + assertThrows(VerificationException.class, () -> ElectrumServer.HeaderSyncService.syncAnnouncedHeaders(new ChainTip(5000, unlinked))); + + ChainTip storeTip = ElectrumServer.getStoreTip(); + assertEquals(12, storeTip.height()); + assertEquals(chain.getLast().getHash(), storeTip.header().getHash()); + } + + /** + * Where verification is mandatory, a server answering that it has no block.headers has not substantiated the tip it announced, so the failure must + * reach the service as itself for the tip to be refused - with the capability still on, which is what lets a history pass rotate the server. A + * private server answering the same has verification turned off for the session instead, and is never refused. + */ + @Test + public void refusesATipFromAMandatoryServerWithoutTheHeadersCall() throws Exception { + Network.set(Network.MAINNET); + ServerCapability previousCapability = ElectrumServer.serverCapability; + ServerType previousServerType = Config.get().getServerType(); + try { + FakeElectrumServerRpc fake = serve(List.of()); + fake.setFailure(new UnsupportedMethodException("blockchain.block.headers", new IllegalStateException())); + ChainTip tip = new ChainTip(Network.MAINNET.getHeaderCheckpoints().getMaxHeight() + 10, Network.MAINNET.getGenesisHeader()); + + ElectrumServer.serverCapability = new ServerCapability(false, false, false); + Config.get().setServerType(ServerType.PUBLIC_ELECTRUM_SERVER); + assertThrows(UnsupportedMethodException.class, () -> ElectrumServer.HeaderSyncService.syncAnnouncedHeaders(tip)); + assertTrue(ElectrumServer.serverCapability.supportsMerkleProofs()); + + Config.get().setServerType(ServerType.ELECTRUM_SERVER); + ElectrumServer.HeaderSyncService.syncAnnouncedHeaders(tip); + assertFalse(ElectrumServer.serverCapability.supportsMerkleProofs()); + } finally { + Config.get().setServerType(previousServerType); + ElectrumServer.serverCapability = previousCapability; + } + } + private static void runOffThread(Callable task) throws Exception { ExecutorService executorService = Executors.newSingleThreadExecutor(); try {