Skip to content

Improve and refactor method to search for stored block in mainchain #574

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

Open
julia-zack wants to merge 2 commits into
base: powpeg-refactors-integration
Choose a base branch
from
Open

Improve and refactor method to search for stored block in mainchain #574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
503d156
Improve and refactor method to search for stored block in mainchain. …
julia-zack May 20, 2026
4defc6d
Added test to check appearance hashes from non-stubed blocks are just…
julia-zack May 21, 2026
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
94 changes: 32 additions & 62 deletions src/main/java/co/rsk/federate/BtcToRskClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,6 @@
}

protected int updateBridgeBtcBlockchain() throws BlockStoreException, IOException {
long bestBlockNumber = federatorSupport.getRskBestChainHeight();
boolean useBlockDepth = activationConfig.isActive(ConsensusRule.RSKIP89, bestBlockNumber);
Comment on lines -388 to -389
Copy link
Copy Markdown
Contributor Author

@julia-zack julia-zack May 21, 2026

Choose a reason for hiding this comment

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

no need to check for activations


int bridgeBtcBlockchainBestChainHeight = federatorSupport.getBtcBlockchainBestChainHeight();
int federatorBtcBlockchainBestChainHeight = bitcoinWrapper.getBestChainHeight();
if (federatorBtcBlockchainBestChainHeight > bridgeBtcBlockchainBestChainHeight) {
Expand All @@ -400,16 +397,12 @@
// update the bridge with the latest.

// First, find the common ancestor that is in the federator's bestchain
// using either the old method -- block locator
// or the new one -- block depth incremental search
StoredBlock commonAncestor;
if (useBlockDepth) {
commonAncestor = findBridgeBtcBlockchainMatchingAncestor(bridgeBtcBlockchainBestChainHeight);
} else {
commonAncestor = findBridgeBtcBlockchainMatchingAncestorUsingBlockLocator();
// using block depth incremental search
Optional<StoredBlock> commonAncestorOpt = findBridgeBtcBlockchainMatchingAncestor(bridgeBtcBlockchainBestChainHeight);
if (commonAncestorOpt.isEmpty()) {
throw new BlockStoreException("No best chain block found");
}

checkNotNull(commonAncestor, "No best chain block found");
StoredBlock commonAncestor = commonAncestorOpt.get();

logger.debug(
"[updateBridgeBtcBlockchain] Matched block {}.",
Expand Down Expand Up @@ -482,7 +475,7 @@
}
}

private StoredBlock findBridgeBtcBlockchainMatchingAncestor(int bridgeBtcBlockchainBestChainHeight) throws BlockStoreException {
private Optional<StoredBlock> findBridgeBtcBlockchainMatchingAncestor(int bridgeBtcBlockchainBestChainHeight) throws BlockStoreException {
// Find the last federator's best chain block the bridge has and update from there
int bridgeBtcBlockchainInitialBlockHeight = federatorSupport.getBtcBlockchainInitialBlockHeight();
int maxSearchDepth = bridgeBtcBlockchainBestChainHeight - bridgeBtcBlockchainInitialBlockHeight;
Expand All @@ -492,27 +485,17 @@
maxSearchDepth
);

StoredBlock matchedBlock = null;
int currentSearchDepth = 0;
int iteration = 0;
while (true) {
Sha256Hash storedBlockHash = federatorSupport.getBtcBlockchainBlockHashAtDepth(currentSearchDepth);
StoredBlock storedBlock = bitcoinWrapper.getBlock(storedBlockHash);
Optional<StoredBlock> storedBlock = getMatchingStoredBlockInMainChain(storedBlockHash);
logger.trace(
"[findBridgeBtcBlockchainMatchingAncestor] block[storedBlockHash] found? {}",
storedBlock != null
storedBlock.isPresent()
);
if (storedBlock != null) {
StoredBlock storedBlockInBestChain = bitcoinWrapper.getBlockAtHeight(storedBlock.getHeight());
logger.trace(
"[findBridgeBtcBlockchainMatchingAncestor] block[{}] in best chain? {}",
storedBlock.getHeader().getHash(),
storedBlock.equals(storedBlockInBestChain)
);
if (storedBlock.equals(storedBlockInBestChain)) {
matchedBlock = storedBlockInBestChain;
break;
}
if (storedBlock.isPresent()) {
return storedBlock;
}

// Have we just searched at maximum depth? If so, no more to search, nothing found!
Expand All @@ -528,36 +511,27 @@
currentSearchDepth = IntStream.of(1 << iteration, maxSearchDepth).min().getAsInt();
iteration++;
}

return matchedBlock;
return Optional.empty();
}

private StoredBlock findBridgeBtcBlockchainMatchingAncestorUsingBlockLocator() throws BlockStoreException {
Copy link
Copy Markdown
Contributor Author

@julia-zack julia-zack May 21, 2026

Choose a reason for hiding this comment

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

removed since only being used pre rskip89

// Find the last best chain block the bridge has with respect
// to the federate node's best chain.
Object[] blockLocatorArray = federatorSupport.getBtcBlockchainBlockLocator();
logger.debug(
"Block locator size {}, first {}, last {}.",
blockLocatorArray.length,
blockLocatorArray[0],
blockLocatorArray[blockLocatorArray.length - 1]
private Optional<StoredBlock> getMatchingStoredBlockInMainChain(Sha256Hash blockHash) throws BlockStoreException {
StoredBlock storedBlock = bitcoinWrapper.getBlock(blockHash);
if (storedBlock == null) {
return Optional.empty();
}
int storedBlockHeight = storedBlock.getHeight();
StoredBlock storedBlockAtHeightInBestChain = bitcoinWrapper.getBlockAtHeight(storedBlockHeight);
boolean isBlockInBestChain = storedBlock.equals(storedBlockAtHeightInBestChain);
logger.trace(
"[getMatchingStoredBlockInMainChain] block[{}] in best chain? {}",
storedBlock.getHeader().getHash(),
isBlockInBestChain
);

StoredBlock matchedBlock = null;
for (Object o : blockLocatorArray) {
String blockHash = (String) o;
StoredBlock storedBlock = bitcoinWrapper.getBlock(Sha256Hash.wrap(blockHash));
if (storedBlock == null) {
continue;
}
StoredBlock storedBlockInBestChain = bitcoinWrapper.getBlockAtHeight(storedBlock.getHeight());
if (storedBlock.equals(storedBlockInBestChain)) {
matchedBlock = storedBlockInBestChain;
break;
}
if (isBlockInBestChain) {
return Optional.of(storedBlockAtHeightInBestChain);
}

return matchedBlock;
return Optional.empty();
}

protected void updateBridgeBtcTransactions() {
Expand All @@ -579,7 +553,7 @@
int numberOfTxsSent = 0;

Iterator<Sha256Hash> txHashIterator = txsToSendToRskHashes.iterator();
while (txHashIterator.hasNext()) {

Check warning on line 556 in src/main/java/co/rsk/federate/BtcToRskClient.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Reduce the total number of break and continue statements in this loop to use at most one. 


See more on https://sonarcloud.io/project/issues?id=rsksmart_powpeg-node&issues=AZ5G1vU6stnyi3a5fIUG&open=AZ5G1vU6stnyi3a5fIUG&pullRequest=574
Sha256Hash txHash = txHashIterator.next();
try {
Transaction tx = federatorWalletTxMap.get(txHash);
Expand All @@ -603,7 +577,7 @@
);

// Check if the tx was processed (using the tx hash without witness)
if (federatorSupport.isBtcTxHashAlreadyProcessed(txId)) {

Check warning on line 580 in src/main/java/co/rsk/federate/BtcToRskClient.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a primitive boolean expression here. 


See more on https://sonarcloud.io/project/issues?id=rsksmart_powpeg-node&issues=AZ5G1vU6stnyi3a5fIUD&open=AZ5G1vU6stnyi3a5fIUD&pullRequest=574
logger.debug(
"[updateBridgeBtcTransactions] Btc Tx {} (wtxid: {}) already processed",
txId,
Expand All @@ -616,7 +590,7 @@
// If M >= N + K, then remove the transaction from the list
Long txProcessedHeight = federatorSupport.getBtcTxHashProcessedHeight(txId);
Long bestChainHeight = federatorSupport.getRskBestChainHeight();
if (bestChainHeight >= txProcessedHeight + BTC_TO_RSK_MINIMUM_ACCEPTABLE_CONFIRMATIONS_ON_RSK) {

Check failure on line 593 in src/main/java/co/rsk/federate/BtcToRskClient.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this code to not nest more than 3 if/for/while/switch/try statements. 


See more on https://sonarcloud.io/project/issues?id=rsksmart_powpeg-node&issues=AZ5G1vU6stnyi3a5fIUF&open=AZ5G1vU6stnyi3a5fIUF&pullRequest=574
removeTxHashFromFile(txHashIterator);
logger.debug(
"[updateBridgeBtcTransactions] Btc Tx {} was processed at height {}, current height is {}. Tx removed from pending lock list",
Expand Down Expand Up @@ -704,8 +678,8 @@

private Optional<StoredBlock> getStoredBlock(Transaction tx) {
try {
return Optional.of(findBestChainStoredBlockFor(tx));
return findBestChainStoredBlockFor(tx);
} catch (BlockStoreException | IllegalStateException e) {

Check warning on line 682 in src/main/java/co/rsk/federate/BtcToRskClient.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Either log or rethrow this exception. 


See more on https://sonarcloud.io/project/issues?id=rsksmart_powpeg-node&issues=AZ5G1vU6stnyi3a5fIUE&open=AZ5G1vU6stnyi3a5fIUE&pullRequest=574
return Optional.empty();
}
}
Expand Down Expand Up @@ -832,24 +806,20 @@

/**
* Finds the block in the best chain where supplied tx appears.
* @throws IllegalStateException If the tx is not in the best chain
*/
private StoredBlock findBestChainStoredBlockFor(Transaction tx) throws IllegalStateException, BlockStoreException {
private Optional<StoredBlock> findBestChainStoredBlockFor(Transaction tx) throws IllegalStateException, BlockStoreException {
Map<Sha256Hash, Integer> blockHashes = tx.getAppearsInHashes();

if (blockHashes != null) {
for (Sha256Hash blockHash : blockHashes.keySet()) {
StoredBlock storedBlock = bitcoinWrapper.getBlock(blockHash);
// Find out if that block is in the main chain
int height = storedBlock.getHeight();
StoredBlock storedBlockAtHeight = bitcoinWrapper.getBlockAtHeight(height);
if (storedBlockAtHeight!=null && storedBlockAtHeight.getHeader().getHash().equals(blockHash)) {
return storedBlockAtHeight;
Optional<StoredBlock> storedBlock = getMatchingStoredBlockInMainChain(blockHash);
if (storedBlock.isPresent()) {
return storedBlock;
}
}
}

throw new IllegalStateException("Tx not in the best chain: " + tx.getWTxId());
return Optional.empty();
}

@PreDestroy
Expand Down
178 changes: 31 additions & 147 deletions src/test/java/co/rsk/federate/BtcToRskClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,149 +685,6 @@
verify(btcToRskClientFileStorageMock, times(1)).write(any());
}

@Test
Copy link
Copy Markdown
Contributor Author

@julia-zack julia-zack May 20, 2026

Choose a reason for hiding this comment

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

removed all pre-rskip89 tests

void updateBlockchainWithoutBlocks_preRskip89() throws Exception {
simulatePreRskip89();

BitcoinWrapper bw = new SimpleBitcoinWrapper();
SimpleFederatorSupport fh = new SimpleFederatorSupport();
BtcToRskClient client = createClientWithMocks(bw, fh);

int numberOfBlocksSent = client.updateBridgeBtcBlockchain();
assertEquals(0, numberOfBlocksSent);

assertNull(fh.getReceiveHeaders());
}

@Test
void updateBlockchainWithBetterBlockchainInContract_preRskip89() throws Exception {
simulatePreRskip89();

BitcoinWrapper bw = new SimpleBitcoinWrapper();
SimpleFederatorSupport fh = new SimpleFederatorSupport();
fh.setBtcBlockchainBestChainHeight(10);
BtcToRskClient client = createClientWithMocks(bw, fh);

int numberOfBlocksSent = client.updateBridgeBtcBlockchain();
assertEquals(0, numberOfBlocksSent);

assertNull(fh.getReceiveHeaders());

assertEquals(0, fh.getSendReceiveHeadersInvocations());
}

@Test
void updateBlockchainWithBetterBlockchainInWalletByOneBlock_preRskip89() throws Exception {
simulatePreRskip89();

SimpleBitcoinWrapper bw = new SimpleBitcoinWrapper();
StoredBlock[] blocks = createBlockchain(3);
bw.setBlocks(blocks);
SimpleFederatorSupport fh = new SimpleFederatorSupport();
fh.setBtcBlockchainBestChainHeight(2);
fh.setBtcBlockchainBlockLocator(createLocator(blocks, 2, 0));
BtcToRskClient client = createClientWithMocks(bw, fh);

int numberOfBlocksSent = client.updateBridgeBtcBlockchain();

Block[] headers = fh.getReceiveHeaders();

assertNotNull(headers);
assertEquals(1, headers.length);
assertEquals(1, numberOfBlocksSent);
assertEquals(blocks[3].getHeader().getHash(), headers[0].getHash());

assertEquals(1, fh.getSendReceiveHeadersInvocations());
}

@Test
void updateBlockchainWithBetterBlockchainInWalletByTwoBlocks_preRskip89() throws Exception {
simulatePreRskip89();

SimpleBitcoinWrapper bw = new SimpleBitcoinWrapper();
StoredBlock[] blocks = createBlockchain(4);
bw.setBlocks(blocks);
SimpleFederatorSupport fh = new SimpleFederatorSupport();
fh.setBtcBlockchainBestChainHeight(2);
fh.setBtcBlockchainBlockLocator(createLocator(blocks, 2, 0));
BtcToRskClient client = createClientWithMocks(bw, fh);

int numberOfBlocksSent = client.updateBridgeBtcBlockchain();

Block[] headers = fh.getReceiveHeaders();

assertNotNull(headers);
assertEquals(2, headers.length);
assertEquals(2, numberOfBlocksSent);
assertEquals(blocks[3].getHeader().getHash(), headers[0].getHash());
assertEquals(blocks[4].getHeader().getHash(), headers[1].getHash());

assertEquals(1, fh.getSendReceiveHeadersInvocations());
}

@Test
void updateBlockchainWithBetterBlockchainInWalletByThreeBlocks_preRskip89() throws Exception {
simulatePreRskip89();

SimpleBitcoinWrapper bw = new SimpleBitcoinWrapper();
StoredBlock[] blocks = createBlockchain(4);
bw.setBlocks(blocks);
SimpleFederatorSupport fh = new SimpleFederatorSupport();
fh.setBtcBlockchainBestChainHeight(2);
fh.setBtcBlockchainBlockLocator(createLocator(blocks, 1, 1));
BtcToRskClient client = createClientWithMocks(bw, fh);

client.updateBridgeBtcBlockchain();

Block[] headers = fh.getReceiveHeaders();

assertNotNull(headers);
assertEquals(3, headers.length);
assertEquals(blocks[2].getHeader().getHash(), headers[0].getHash());
assertEquals(blocks[3].getHeader().getHash(), headers[1].getHash());
assertEquals(blocks[4].getHeader().getHash(), headers[2].getHash());

assertEquals(1, fh.getSendReceiveHeadersInvocations());
}

@Test
void updateBlockchainWithBetterBlockchainInWalletBySixHundredBlocks_preRskip89() throws Exception {
simulatePreRskip89();

StoredBlock[] blocks = createBlockchain(601);

SimpleBitcoinWrapper bitcoinWrapper = new SimpleBitcoinWrapper();
bitcoinWrapper.setBlocks(blocks);

federatorSupport.setBtcBlockchainBestChainHeight(1);
federatorSupport.setBtcBlockchainBlockLocator(createLocator(blocks, 1, 0));

BtcLockSenderProvider btcLockSenderProvider = mockBtcLockSenderProvider(TxSenderAddressType.P2PKH);
int amountOfHeadersToSend = 345;

PowpegNodeSystemProperties config = mock(PowpegNodeSystemProperties.class);
when(config.getAmountOfHeadersToSend()).thenReturn(amountOfHeadersToSend);

BtcToRskClient client = btcToRskClientBuilder
.withActivationConfig(activationConfig)
.withBitcoinWrapper(bitcoinWrapper)
.withFederatorSupport(federatorSupport)
.withBridgeConstants(bridgeRegTestConstants)
.withBtcLockSenderProvider(btcLockSenderProvider)
.withFederation(activeFederation)
.withFedNodeSystemProperties(config)
.build();

client.updateBridgeBtcBlockchain();

Block[] headers = federatorSupport.getReceiveHeaders();

assertNotNull(headers);
assertEquals(amountOfHeadersToSend, headers.length);

assertEquals(1, federatorSupport.getSendReceiveHeadersInvocations());
}

@Test
void updateBlockchainWithoutBlocks() throws Exception {
BitcoinWrapper bw = new SimpleBitcoinWrapper();
Expand Down Expand Up @@ -3637,6 +3494,37 @@
assertWTxIdIsNotInActiveFedClientProofsFile(peginBtcTx);
}

@ParameterizedTest
@MethodSource("activeFedArgs")
void updateBridgeBtcTransactions_whenOneAppearanceHashNotInBlockStore_shouldBeInformedWithBestChainProof(
Federation federation
) throws Exception {
// arrange
setUpActiveFedClient(federation);
var peginBtcTx = createTxFromP2pkh(MAINNET_BTC_PARAMS);
addOutputToFedWithMinimumPeginValue(peginBtcTx, federation.getAddress());

setUpTx(activeFedClient, peginBtcTx);

// inject other appearance hash that is NOT stubbed in the block store, and
// that iterates first in the natural-ordered treemap
String hashThatWillAppearFirst = "aaaaa00000aaaaa00000aaaaa00000aaaaa00000aaaaa00000aaaaa00000aaaa";
Sha256Hash appearanceHashFromMissingBlock = Sha256Hash.wrap(hashThatWillAppearFirst);
Transaction peginTx = walletTxs.iterator().next();
peginTx.addBlockAppearance(appearanceHashFromMissingBlock, 1);
// assert the block won't be found
assertNull(bitcoinWrapper.getBlock(appearanceHashFromMissingBlock));
// and that its hash is the first one in the treemap
Sha256Hash firstAppearanceHash = peginTx.getAppearsInHashes().keySet().iterator().next();
assertEquals(appearanceHashFromMissingBlock, firstAppearanceHash);

// act
activeFedClient.updateBridgeBtcTransactions();

// assert
assertTxSentToBridgeByActiveFedClient(peginBtcTx);
}

private void assertWTxIdIsInActiveFedClientProofsFile(BtcTransaction btcTx) throws IOException {
Transaction tx = ThinConverter.toOriginalInstance(MAINNET_BTC_PARAMS_STRING, btcTx);
assertWTxIdIsInProofsFile(MAINNET_PARAMS, btcToRskActiveFedClientFileStorage, tx);
Expand Down Expand Up @@ -3950,7 +3838,7 @@
@Test
void updateBridge_whenFederationIsNull_shouldDoNothing() throws Exception {
// Arrange
FederatorSupport federatorSupport = mock(FederatorSupport.class);

Check warning on line 3841 in src/test/java/co/rsk/federate/BtcToRskClientTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Rename "federatorSupport" which hides the field declared at line 69. 


See more on https://sonarcloud.io/project/issues?id=rsksmart_powpeg-node&issues=AZ5G1vWvstnyi3a5fIUO&open=AZ5G1vWvstnyi3a5fIUO&pullRequest=574

BtcToRskClient btcToRskClient = BtcToRskClientBuilder.builder()
.withBridgeConstants(BridgeMainNetConstants.getInstance())
Expand Down Expand Up @@ -4400,10 +4288,6 @@
return tx;
}

private void simulatePreRskip89() {
when(activationConfig.isActive(eq(ConsensusRule.RSKIP89), anyLong())).thenReturn(false);
}

private BtcLockSenderProvider mockBtcLockSenderProvider(TxSenderAddressType txSenderAddressType) {
BtcLockSender btcLockSender = mock(BtcLockSender.class);
when(btcLockSender.getTxSenderAddressType()).thenReturn(txSenderAddressType);
Expand Down
Loading