Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: address several sync issues and remove a potential deadlock #247

Merged
merged 5 commits into from
Mar 8, 2024
Merged

fix: address several sync issues and remove a potential deadlock #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6224302
tests: add bootstrap mainnet files and tests
HashEngineering Mar 1, 2024
18fb28b
chore: Release 20.0.2
HashEngineering Mar 1, 2024
b3e0660
fix: improve synchronization related items
HashEngineering Mar 1, 2024
ac8f7f9
fix: use user thread in for listeners in ChainLocksHandler and Instan…
HashEngineering Mar 5, 2024
4fb46e0
fix: adjust peer connected and disconnected events and log setting do…
HashEngineering Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/org/bitcoinj/core/PeerGroup.java
View file
Open in desktop
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
Open in desktop
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();
Comment on lines -158 to +157
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a problem where v20 bootstrap files cannot be loaded.

} 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);
Comment on lines -533 to +530
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If newBestBlockListener is running in SAME_THREAD, then it will be running under an AbstractBlockChain lock which may be a cause of the deadlock I am trying to fix.

This is the only listener running in the same thread.

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
Open in desktop
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());
}
Comment on lines +1238 to +1244
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the goal here is to count qrinfo messages in the bandwidth calculations.

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
Binary file not shown.
Binary file added core/src/test/resources/org/bitcoinj/quorums/qrinfo-mainnet-0-2028764-70230.dat
View file
Open in desktop
Binary file not shown.
Loading