Skip to content

Commit

Permalink
fix: address several sync issues and remove a potential deadlock (#247)
Browse files Browse the repository at this point in the history 
* tests: add bootstrap mainnet files and tests

Signed-off-by: HashEngineering <[email protected]>

* fix: improve synchronization related items

* improve logging with sendRequestWithRetry

* count mnlistdiff size on Request not Complete

* add fulfilled field to lastRequest and use it

Signed-off-by: HashEngineering <[email protected]>

* fix: use user thread in for listeners in ChainLocksHandler and InstantSendManager

Signed-off-by: HashEngineering <[email protected]>

* fix: adjust peer connected and disconnected events and log setting downloadPeer in AbstractQuorumState

Signed-off-by: HashEngineering <[email protected]>

---------

Signed-off-by: HashEngineering <[email protected]>
  • Loading branch information
@HashEngineering
HashEngineering authored Mar 8, 2024
1 parent 76b7d6c commit 5805914
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 42 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/org/bitcoinj/core/PeerGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,7 @@ public void onPreBlocksDownload(Peer peer) {

@Override
public void onMasterNodeListDiffDownloaded(Stage stage, SimplifiedMasternodeListDiff mnlistdiff) {
if (stage == Stage.Finished) {
if (stage == Stage.Received) {
masternodeListsInLastSecond++;
bytesInLastSecond += mnlistdiff.getMessageSize();
}
Expand Down
44 changes: 20 additions & 24 deletions core/src/main/java/org/bitcoinj/evolution/AbstractQuorumState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import org.bitcoinj.quorums.QuorumRotationInfo;
import org.bitcoinj.quorums.SigningManager;
import org.bitcoinj.quorums.SimplifiedQuorumList;
import org.bitcoinj.store.BlockStore;
import org.bitcoinj.store.BlockStoreException;
import org.bitcoinj.utils.Threading;
import org.slf4j.Logger;
Expand Down Expand Up @@ -155,7 +154,7 @@ public void setBootstrap(String bootstrapFilePath, InputStream bootstrapStream,
this.bootstrapStream = bootstrapStream;
this.bootStrapFileFormat = bootStrapFileFormat;
if (bootStrapFileFormat == SimplifiedMasternodeListManager.SMLE_VERSION_FORMAT_VERSION) {
protocolVersion = NetworkParameters.ProtocolVersion.SMNLE_VERSIONED.getBitcoinProtocolVersion();
protocolVersion = NetworkParameters.ProtocolVersion.CURRENT.getBitcoinProtocolVersion();
} else if (bootStrapFileFormat == SimplifiedMasternodeListManager.BLS_SCHEME_FORMAT_VERSION) {
protocolVersion = NetworkParameters.ProtocolVersion.DMN_TYPE.getBitcoinProtocolVersion();
} else if (bootStrapFileFormat == SimplifiedMasternodeListManager.QUORUM_ROTATION_FORMAT_VERSION) {
Expand All @@ -170,7 +169,6 @@ public void setBlockChain(PeerGroup peerGroup, DualBlockChain blockChain) {
this.blockChain = blockChain;
if (peerGroup != null) {
this.peerGroup = peerGroup;
// peerGroup.addMnListDownloadCompleteListener(() -> initChainTipSyncComplete = true, Threading.SAME_THREAD);
}
}

Expand Down Expand Up @@ -336,9 +334,9 @@ void requestNextMNListDiff() {
if (!shouldProcessMNListDiff())
return;

log.info("download peer = {}", downloadPeer);
log.info("download peer = {}, but obtaining backup from peerGroup downloadPeer", downloadPeer);
Peer downloadPeerBackup = downloadPeer == null ? context.peerGroup.getDownloadPeer() : downloadPeer;

log.info("backup download peer = {}", downloadPeerBackup);
lock.lock();
try {
if (waitingForMNListDiff)
Expand Down Expand Up @@ -408,9 +406,6 @@ void requestNextMNListDiff() {
waitingForMNListDiff = true;
} else {
log.info("there are no pending blocks to process");
//if (!initChainTipSyncComplete) {
// initChainTipSyncComplete = true;
//}
}
} else {
log.warn("downloadPeer is null, not requesting update");
Expand All @@ -425,7 +420,9 @@ void maybeGetMNListDiffFresh() {
return;

if (downloadPeer == null) {
log.info("using peerGroup downloadPeer in maybeGetMNListDiffFresh ");
downloadPeer = context.peerGroup.getDownloadPeer();
log.info("using peerGroup downloadPeer in maybeGetMNListDiffFresh {}", downloadPeer);
}

lock.lock();
Expand Down Expand Up @@ -530,7 +527,7 @@ public boolean isDeterministicMNsSporkActive() {
}

public void addEventListeners(AbstractBlockChain blockChain, PeerGroup peerGroup) {
blockChain.addNewBestBlockListener(Threading.SAME_THREAD, newBestBlockListener);
blockChain.addNewBestBlockListener(newBestBlockListener);
blockChain.addReorganizeListener(reorganizeListener);
if (peerGroup != null) {
peerGroup.addConnectedEventListener(peerConnectedEventListener);
Expand Down Expand Up @@ -588,24 +585,21 @@ public void notifyNewBestBlock(StoredBlock block) throws VerificationException {
public final PeerConnectedEventListener peerConnectedEventListener = new PeerConnectedEventListener() {
@Override
public void onPeerConnected(Peer peer, int peerCount) {
lock.lock();
try {
if (downloadPeer == null)
downloadPeer = peer;
} finally {
lock.unlock();
}
downloadPeer = context.peerGroup.getDownloadPeer();
log.info("peer connected and setting download peer to {} with onPeerConnected", downloadPeer);
}
};

final PeerDisconnectedEventListener peerDisconnectedEventListener = new PeerDisconnectedEventListener() {
@Override
public void onPeerDisconnected(Peer peer, int peerCount) {
if (downloadPeer == peer) {
downloadPeer = null;
chooseRandomDownloadPeer();
downloadPeer = context.peerGroup.getDownloadPeer();
log.info("setting download peer to {} with onPeerDisconnected, previously was {}", downloadPeer, peer);
if (downloadPeer == null)
chooseRandomDownloadPeer();
}
if (peer.getAddress().equals(lastRequest.getPeerAddress())) {
if (peer.getAddress().equals(lastRequest.getPeerAddress()) && lastRequest.isFullfilled()) {
log.warn("Disconnecting from peer {} before processing mnlistdiff", peer.getAddress());
// TODO: what else should we do?
// request again?
Expand Down Expand Up @@ -650,6 +644,7 @@ void chooseRandomDownloadPeer() {
List<Peer> peers = context.peerGroup.getConnectedPeers();
if (peers != null && !peers.isEmpty()) {
downloadPeer = peers.get(new Random().nextInt(peers.size()));
log.info("setting download peer with chooseRandomDownloadPeer: {}", downloadPeer);
}
}

Expand All @@ -659,6 +654,7 @@ public void onChainDownloadStarted(Peer peer, int blocksLeft) {
lock.lock();
try {
downloadPeer = peer;
log.info("setting download peer with onChainDownloadStarted {}", peer);
// perhaps this is not required with headers first sync
// does this need to be in the next listener?
if (stateManager.isLoadedFromFile())
Expand All @@ -675,6 +671,7 @@ public void onChainDownloadStarted(Peer peer, int blocksLeft) {
public void onHeadersDownloadStarted(Peer peer, int blocksLeft) {
lock.lock();
try {
log.info("setting download peer with onHeadersDownloadStarted: {}", peer);
downloadPeer = peer;
} finally {
lock.unlock();
Expand Down Expand Up @@ -716,14 +713,14 @@ public void run() {
} catch (ExecutionException e) {
// send the message again
try {
log.info("Exception when sending {}", lastRequest.getRequestMessage().getClass().getSimpleName(), e);
log.info("Exception when sending {} to {}", lastRequest.getRequestMessage().getClass().getSimpleName(), peer, e);

// use tryLock to avoid deadlocks
boolean isLocked = context.peerGroup.getLock().tryLock(500, TimeUnit.MILLISECONDS);
try {
if (isLocked) {
log.info(Thread.currentThread().getName() + ": lock acquired");
downloadPeer = context.peerGroup.getDownloadPeer();
log.info(Thread.currentThread().getName() + ": lock acquired, obtaining downloadPeer from peerGroup: {}", downloadPeer);
if (downloadPeer == null) {
chooseRandomDownloadPeer();
}
Expand All @@ -735,12 +732,12 @@ public void run() {
}
}
} catch (InterruptedException x) {
x.printStackTrace();
log.info("sendMessageFuture interrupted", x);
} catch (NullPointerException x) {
log.info("peergroup is not initialized", x);
}
} catch (InterruptedException e) {
e.printStackTrace();
log.info("sendMessageFuture interrupted", e);
}
}
}, Threading.THREAD_POOL);
Expand Down Expand Up @@ -839,7 +836,6 @@ Sha256Hash getHashModifier(LLMQParameters llmqParams, StoredBlock quorumBaseBloc
if (params.isV20Active(workBlock.getHeight())) {
// v20 is active: calculate modifier using the new way.
BLSSignature bestCLSignature = getCoinbaseChainlock(workBlock);
log.info("getHashModifier(..., {})\n work: {}\n sig: {}", quorumBaseBlock.getHeader().getHash(), workBlock.getHeader().getHash(), bestCLSignature);
if (bestCLSignature != null) {
// We have a non-null CL signature: calculate modifier using this CL signature

Expand Down
14 changes: 9 additions & 5 deletions core/src/main/java/org/bitcoinj/evolution/QuorumRotationState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,12 @@ public void applyDiff(Peer peer, DualBlockChain blockChain,
StoredBlock blockMinus3C;
StoredBlock blockMinus4C = null;
long newHeight = ((CoinbaseTx) quorumRotationInfo.getMnListDiffAtH().coinBaseTx.getExtraPayloadObject()).getHeight();
if (peer != null)
peer.queueMasternodeListDownloadedListeners(MasternodeListDownloadedListener.Stage.Received, quorumRotationInfo.getMnListDiffTip());

boolean isSyncingHeadersFirst = context.peerGroup != null && context.peerGroup.getSyncStage() == PeerGroup.SyncStage.MNLIST;

log.info("processing {} qrinfo between (atH): {} & {}; {}",
log.info("processing {} qrinfo between (atH): {} & {}; {} from {}",
isLoadingBootStrap ? "bootstrap" : "requested",
mnListAtH.getHeight(), newHeight, quorumRotationInfo.toString(blockChain));
mnListAtH.getHeight(), newHeight, quorumRotationInfo.toString(blockChain), peer);

blockAtTip = blockChain.getBlock(quorumRotationInfo.getMnListDiffTip().blockHash);
blockAtH = blockChain.getBlock(quorumRotationInfo.getMnListDiffAtH().blockHash);
Expand Down Expand Up @@ -1237,8 +1235,13 @@ public String toString() {
public void processDiff(@Nullable Peer peer, QuorumRotationInfo quorumRotationInfo, DualBlockChain blockChain,
boolean isLoadingBootStrap, PeerGroup.SyncStage syncStage) throws VerificationException {
long newHeight = ((CoinbaseTx) quorumRotationInfo.getMnListDiffTip().coinBaseTx.getExtraPayloadObject()).getHeight();
if (peer != null)
if (peer != null) {
peer.queueMasternodeListDownloadedListeners(MasternodeListDownloadedListener.Stage.Received, quorumRotationInfo.getMnListDiffTip());
peer.queueMasternodeListDownloadedListeners(MasternodeListDownloadedListener.Stage.Received, quorumRotationInfo.getMnListDiffAtH());
peer.queueMasternodeListDownloadedListeners(MasternodeListDownloadedListener.Stage.Received, quorumRotationInfo.getMnListDiffAtHMinusC());
peer.queueMasternodeListDownloadedListeners(MasternodeListDownloadedListener.Stage.Received, quorumRotationInfo.getMnListDiffAtHMinus2C());
peer.queueMasternodeListDownloadedListeners(MasternodeListDownloadedListener.Stage.Received, quorumRotationInfo.getMnListDiffAtHMinus3C());
}
Stopwatch watch = Stopwatch.createStarted();
boolean isSyncingHeadersFirst = syncStage == PeerGroup.SyncStage.MNLIST;

Expand All @@ -1251,6 +1254,7 @@ public void processDiff(@Nullable Peer peer, QuorumRotationInfo quorumRotationIn

unCache();
failedAttempts = 0;
lastRequest.setFulfilled();

if (!pendingBlocks.isEmpty()) {
pendingBlocks.pop();
Expand Down
9 changes: 5 additions & 4 deletions core/src/main/java/org/bitcoinj/evolution/QuorumState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ public void processDiff(@Nullable Peer peer, SimplifiedMasternodeListDiff mnlist
Stopwatch watchMNList = Stopwatch.createUnstarted();
Stopwatch watchQuorums = Stopwatch.createUnstarted();
boolean isSyncingHeadersFirst = context.peerGroup != null && context.peerGroup.getSyncStage() == PeerGroup.SyncStage.MNLIST;
log.info("processing {} mnlistdiff between : {} & {}; {}",
isLoadingBootStrap ? "bootstrap" : "requested",
getMnList().getHeight(), newHeight, mnlistdiff);
log.info("processing {} mnlistdiff (headersFirst={}) between : {} & {}; {} from {}",
isLoadingBootStrap ? "bootstrap" : "requested", isSyncingHeadersFirst,
getMnList().getHeight(), newHeight, mnlistdiff, peer);

mnlistdiff.dump(mnList.getHeight(), newHeight);

Expand All @@ -270,6 +270,7 @@ public void processDiff(@Nullable Peer peer, SimplifiedMasternodeListDiff mnlist
applyDiff(peer, blockChain, mnlistdiff, isLoadingBootStrap);

log.info(this.toString());
lastRequest.setFulfilled();
unCache();
clearFailedAttempts();

Expand Down Expand Up @@ -329,7 +330,7 @@ public void processDiff(@Nullable Peer peer, SimplifiedMasternodeListDiff mnlist
watch.stop();
log.info("processing mnlistdiff times : Total: " + watch + "mnList: " + watchMNList + " quorums" + watchQuorums + "mnlistdiff" + mnlistdiff);
waitingForMNListDiff = false;
if (!initChainTipSyncComplete()) {
if (!initChainTipSyncComplete() && !isLoadingBootStrap) {
log.info("initChainTipSync=false");
context.peerGroup.triggerMnListDownloadComplete();
log.info("initChainTipSync=true");
Expand Down
9 changes: 9 additions & 0 deletions core/src/main/java/org/bitcoinj/evolution/QuorumUpdateRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
public class QuorumUpdateRequest<T extends AbstractQuorumRequest> {
T request;
long time;
private boolean fulfilled = false;

private PeerAddress peerAddress;
public QuorumUpdateRequest(T request) {
Expand Down Expand Up @@ -76,4 +77,12 @@ public String toString(DualBlockChain blockChain) {
", time=" + time +
'}';
}

public void setFulfilled() {
this.fulfilled = true;
}

public boolean isFullfilled() {
return fulfilled;
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/bitcoinj/quorums/ChainLocksHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public ChainLocksHandler(Context context) {
public void setBlockChain(AbstractBlockChain blockChain, AbstractBlockChain headerChain) {
this.blockChain = blockChain;
this.headerChain = headerChain;
this.blockChain.addNewBestBlockListener(Threading.SAME_THREAD, this.newBestBlockListener);
this.blockChain.addNewBestBlockListener(this.newBestBlockListener);
this.quorumSigningManager = context.signingManager;
this.quorumInstantSendManager = context.instantSendManager;
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ public InstantSendManager(Context context, InstantSendDatabase db, boolean runWi
public void setBlockChain(AbstractBlockChain blockChain, @Nullable PeerGroup peerGroup) {
this.blockChain = blockChain;
this.blockChain.addTransactionReceivedListener(this.transactionReceivedInBlockListener);
this.blockChain.addNewBestBlockListener(Threading.SAME_THREAD, this.newBestBlockListener);
this.blockChain.addNewBestBlockListener(this.newBestBlockListener);
if (peerGroup != null) {
peerGroup.addOnTransactionBroadcastListener(this.transactionBroadcastListener);
}
context.chainLockHandler.addChainLockListener(this.chainLockListener, Threading.SAME_THREAD);
context.chainLockHandler.addChainLockListener(this.chainLockListener);
}

public void close(PeerGroup peerGroup) {
Expand Down
10 changes: 5 additions & 5 deletions core/src/test/java/org/bitcoinj/evolution/LoadBootstrapFilesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ public static Collection<Object[]> data() {
1888408
},
{
TESTNETPARAMS,
"mnlistdiff-testnet-0-850798-70228-after19.2HF.dat",
"qrinfo-testnet-0-850806-70228-after19.2HF.dat",
MAINPARAMS,
"mnlistdiff-mainnet-0-2028691-70230.dat",
"qrinfo-mainnet-0-2028764-70230.dat",
SimplifiedMasternodeListManager.SMLE_VERSION_FORMAT_VERSION,
850798,
850744
2028691,
2028664
}
});
}
Expand Down
15 changes: 15 additions & 0 deletions core/src/test/java/org/bitcoinj/evolution/SimplifiedMasternodeListDiffTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,19 @@ public void mnlistdiff_70228_afterActivation() throws IOException {

assertArrayEquals(payloadOne, mnlistdiff.bitcoinSerialize());
}

@Test
public void mnlistdiff_70230() throws IOException {
BLSScheme.setLegacyDefault(true); // the qrinfo will set the scheme to basic
payloadOne = loadMnListDiff("mnlistdiff-mainnet-0-2028691-70230.dat");
SimplifiedMasternodeListDiff mnlistdiff = new SimplifiedMasternodeListDiff(PARAMS, payloadOne, 70230);
assertArrayEquals(payloadOne, mnlistdiff.bitcoinSerialize());

assertTrue(mnlistdiff.hasChanges());
assertEquals(Sha256Hash.wrap("000000000000000f78a0addf3f9a4c65a4d0f2ca8e63d5893f8227e1585ef3d8"), mnlistdiff.blockHash);
assertEquals(1, mnlistdiff.getVersion());
assertTrue(mnlistdiff.hasBasicSchemeKeys());

assertArrayEquals(payloadOne, mnlistdiff.bitcoinSerialize());
}
}
14 changes: 14 additions & 0 deletions core/src/test/java/org/bitcoinj/quorums/QuorumRotationInfoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,18 @@ public void qrinfo_70230_afterActivation() throws IOException {

assertArrayEquals(payloadOne, quorumRotationInfo.bitcoinSerialize());
}

@Test
public void qrinfo_70230() throws IOException {
payloadOne = loadQRInfo("qrinfo-mainnet-0-2028764-70230.dat");
QuorumRotationInfo quorumRotationInfo = new QuorumRotationInfo(MAINNET, payloadOne, 702230);
assertArrayEquals(payloadOne, quorumRotationInfo.bitcoinSerialize());

assertTrue(quorumRotationInfo.hasChanges());
assertEquals(Sha256Hash.wrap("00000000000000239004bad185d58602b8b90cc8211d29f55b93d72bdaa3a098"), quorumRotationInfo.mnListDiffTip.blockHash);
assertEquals(SimplifiedMasternodeListDiff.LEGACY_BLS_VERSION, quorumRotationInfo.mnListDiffAtH.getVersion());
assertTrue(quorumRotationInfo.mnListDiffAtH.hasBasicSchemeKeys());

assertArrayEquals(payloadOne, quorumRotationInfo.bitcoinSerialize());
}
}
Binary file added core/src/test/resources/org/bitcoinj/evolution/mnlistdiff-mainnet-0-2028691-70230.dat
View file
Binary file not shown.
Binary file added core/src/test/resources/org/bitcoinj/quorums/qrinfo-mainnet-0-2028764-70230.dat
View file
Binary file not shown.

0 comments on commit 5805914

Please sign in to comment.