Skip to content

Added a test to check what is the maximum number of inputs allowed #3405

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
julianlen wants to merge 3 commits into
base: mtmu-integration
Choose a base branch
from
Open

Added a test to check what is the maximum number of inputs allowed #3405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fbd0d3
added a test to check what is the maximum number of inputs allowed
julianlen Dec 5, 2025
bccc44d
renamings, tests improvements, and fixing typos
julianlen Dec 12, 2025
649ea08
little renaming
julianlen Dec 12, 2025
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
32 changes: 29 additions & 3 deletions rskj-core/src/main/java/co/rsk/peg/BridgeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,33 @@ private static int calculateLegacyTxSize(Federation federation, int inputsCount,
return baseSize + signingSize;
}

public static int calculateBtcTxVirtualSize(BtcTransaction btcTx) {
int baseSize = 0;
List<TransactionInput> inputs = btcTx.getInputs();
baseSize += 1; // input count
for (TransactionInput input : inputs) {
baseSize += input.bitcoinSerialize().length;
}

List<TransactionOutput> outputs = btcTx.getOutputs();
baseSize += 1; // output count
for (TransactionOutput output : outputs) {
baseSize += output.bitcoinSerialize().length;
}

baseSize += 4; // version
baseSize += 4; // locktime

int totalSize = btcTx.bitcoinSerialize().length;
return calculateVirtualBytes(totalSize, baseSize);
}

// As described in BIP141
private static int calculateVirtualBytes(int totalSize, int baseSize) {
int txWeight = totalSize + (baseSize * 3);
return txWeight / 4;
}

private static int calculateSegwitTxSize(Federation federation, int inputsCount, int outputsCount) {
BtcTransaction tx = new BtcTransaction(federation.getBtcParams());

Expand All @@ -681,9 +708,8 @@ private static int calculateSegwitTxSize(Federation federation, int inputsCount,
int baseSize = calculateTxBaseSize(tx, inputsCount, true);
int signingSize = getSigningSize(federation.getNumberOfSignaturesRequired(), inputsCount);
int totalSize = baseSize + signingSize + (inputsCount * federation.getRedeemScript().getProgram().length);
// As described in BIP141
int txWeight = totalSize + (3 * baseSize);
return txWeight / 4;

return calculateVirtualBytes(totalSize, baseSize);
}

private static int getSigningSize(int numberOfSignaturesRequired, int inputsCount) {
Expand Down
6 changes: 3 additions & 3 deletions rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.junit.jupiter.api.*;

class BridgeSupportSvpTest {
private static final byte[] EMPTY_SCRIPT_SIG = new byte[]{};
private static final ActivationConfig.ForBlock allActivations = ActivationConfigsForTest.all().forBlock(0);
private static final RskAddress bridgeContractAddress = PrecompiledContracts.BRIDGE_ADDR;
private static final BridgeConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance();
Expand Down Expand Up @@ -650,7 +651,7 @@ void registerBtcTransaction_forNormalPegout_whenSvpPeriodIsOngoing_shouldRegiste

BtcTransaction pegout = PegoutTransactionBuilder.builder()
.withActiveFederation(activeFederation)
.withInput(BitcoinTestUtils.createHash(2), 0, Coin.COIN)
.withInput(BitcoinTestUtils.createHash(2), 0, Coin.COIN, EMPTY_SCRIPT_SIG)
.withChangeAmount(Coin.COIN.multiply(10))
.withSignatures(activeFederationKeys)
.build();
Expand Down Expand Up @@ -1021,10 +1022,9 @@ void registerBtcTransaction_forPegout_whenWaitingForSvpSpendTx_shouldProcessAndR
// arrange
arrangeSvpSpendTransaction();
setUpForTransactionRegistration(svpSpendTransaction);

BtcTransaction pegout = PegoutTransactionBuilder.builder()
.withActiveFederation(activeFederation)
.withInput(BitcoinTestUtils.createHash(2), 0, Coin.COIN)
.withInput(BitcoinTestUtils.createHash(2), 0, Coin.COIN, EMPTY_SCRIPT_SIG)
.withChangeAmount(Coin.COIN.multiply(10))
.withSignatures(activeFederationKeys)
.build();
Expand Down
16 changes: 16 additions & 0 deletions rskj-core/src/test/java/co/rsk/peg/BridgeUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import java.util.Collections;
import java.util.List;

import static co.rsk.peg.BridgeUtils.calculateBtcTxVirtualSize;
import static co.rsk.peg.PegUtils.getFlyoverFederationOutputScript;
import static co.rsk.peg.bitcoin.BitcoinTestUtils.generateSignerEncodedSignatures;
import static co.rsk.peg.bitcoin.BitcoinTestUtils.generateTransactionInputsSigHashes;
Expand Down Expand Up @@ -2130,4 +2131,19 @@ void testGetUTXOsSentToAddresses_no_utxo_sent_to_given_address_after_RSKIP293()

Assertions.assertTrue(foundUTXOs.isEmpty());
}

@Test
void calculateBtcTxVirtualSize_withARealBtcTx_shouldReturnCorrectVirtualSize() {
// arrange
// https://mempool.space/tx/6f8af5e623c82b9a8e1806d677c57f81118a1da3e44032ec1a2f3603c90a7749
byte[] btcRawTx = Hex.decode("02000000000103762640beb71fcf0d15a61a825c7787c3f60dabf010df4686d3bec4f3c6d2364f02000000232200202bd90b1cbb2ef3b7cb4e3909dfea9b91de17ebc9c04db0ad243fc0358708b928ffffffff1db227830f151ef617fb0aaa39adb6bf30a3b53debeabffcc54a9cb2a836035100000000232200202bd90b1cbb2ef3b7cb4e3909dfea9b91de17ebc9c04db0ad243fc0358708b928ffffffff5af576578a2d88dad6e732a3be1ba82d29cf69211d4cbf342f8fcec31b1ad47501000000232200202bd90b1cbb2ef3b7cb4e3909dfea9b91de17ebc9c04db0ad243fc0358708b928ffffffff018ec62a000000000017a91484708b5f82a7c6b1f1a2274d2d801a9b86fa330d870800483045022100fbd02d4aa6d457732c1b0aaf62a2d9464510efb8d84d548bbf2020c208739fd6022077e6096df604f8beeed4e1eadf56c40cd917337f922054ab2e56ed1ad49ab87e014730440220636fedd886447bfc7181c1c6d6a207bf78cc126a14133d057c3b0927d084c6880220101a21167888382ca3680ffe69bd8a4e3da844a61bbdb0a7246eb294289740c601483045022100b5151837b89cf5a23475870a72d1b5957a7808796c373d99bb6b0e2436fb4e5a02201022c48104723a9a50c323efbaf6764f9adb49292121a7e05294807a9fa4df58014730440220673cd56a052eebfea76661488aa9f05beea12860442040e54fa0d1f0ada94362022025c547e82fcbb4f4eec15f55c17357caf33b5e1d4e72763a671f0634e4f3b6e601483045022100cbf313d9e9d1b75f73fe626fd6450c1910cbe3f0cb2a6d7a571f653c3d588aa30220705879beda53624e7ad16e396dc15f0b6c1363984795f105a9f758756c43aa0c0100fdc901645521026df77fe41e8ac503ba47cb3a27e12661c5ee9d7f9f185d11c5680c0923356c3e21027279e6b34e38bca7c2b05524eb942beb8e5ef4a1e88cadb366eec66128e3036021027ec0acfd4ae4af03dc9559e0508d98b1790607165d8f95a1d4dcd4da47c05743210285409152cf9031c098a968cde3bcc1e85e4135f0dc8bc022d903a0395a84385221028c561a89624fa81819167ecf6175703c5760fce9a7df6703fa0589050c8f02522102b0db2c66fbad3a46f2b0a617660a66ad72f5391aec659dd4b4de5e45d642e40421031c749a4e732bf871ec985496431b71d85c533690c12a4228143cc290c928078f21031d881aabb972128028fc1b830921f1cc9009a51629c42dd59c7582bfef19a90121036613ff4a2959177ab44913d0a900f9917dab43161b126113d6a098994330330d59ae670350cd00b275532102370a9838e4d15708ad14a104ee5606b36caaaaf739d833e67770ce9fd9b3ec80210257c293086c4d4fe8943deda5f890a37d11bebd140e220faa76258a41d077b4d42103c2660a46aa73078ee6016dee953488566426cf55fc8011edd0085634d75395f92103cd3e383ec6e12719a6c69515e5559bcbe037d0aa24c187e1e26ce932e22ad7b354ae6808004730440220592d8757774639dc57e422186ec96185a895ba02ac67db9f393a5387492d6e6202206ed71c353d423e8e2ace536d51697b49bb957907a8a1d5a2ace091022e8221e601483045022100c1ea2f7de1884617f59b399c43695073bb0368e1299224e2b75a6cf67205765a022021f97603c075f7835fdae892a5fe3e5d8db00edb6d62c30ac82debc61103852b0147304402204db4235f92b4279dd839b092f863c7a93396997f00f2e297ff5f2dba7e93180c02205c147fab59087e7e23f46c154c08610e62e6d1ba1bcebf1136eba5084804a03001473044022030d8aab9bee2ceba4983d7def269080470ed283bdd9d8f7508b166195e02576b02201e09d3080c6306c1274bfcc5687581df737efbb20b91256be908bdb49c4c44b601483045022100d517a452532c13d04c2f8f2843d523f818ed73d5b23a43becf295ca4ddbbe69802202d7595ad110459b2078ebf94c68cbbba0060b0f2df3b7cbe40ad20e26dbd06c70100fdc901645521026df77fe41e8ac503ba47cb3a27e12661c5ee9d7f9f185d11c5680c0923356c3e21027279e6b34e38bca7c2b05524eb942beb8e5ef4a1e88cadb366eec66128e3036021027ec0acfd4ae4af03dc9559e0508d98b1790607165d8f95a1d4dcd4da47c05743210285409152cf9031c098a968cde3bcc1e85e4135f0dc8bc022d903a0395a84385221028c561a89624fa81819167ecf6175703c5760fce9a7df6703fa0589050c8f02522102b0db2c66fbad3a46f2b0a617660a66ad72f5391aec659dd4b4de5e45d642e40421031c749a4e732bf871ec985496431b71d85c533690c12a4228143cc290c928078f21031d881aabb972128028fc1b830921f1cc9009a51629c42dd59c7582bfef19a90121036613ff4a2959177ab44913d0a900f9917dab43161b126113d6a098994330330d59ae670350cd00b275532102370a9838e4d15708ad14a104ee5606b36caaaaf739d833e67770ce9fd9b3ec80210257c293086c4d4fe8943deda5f890a37d11bebd140e220faa76258a41d077b4d42103c2660a46aa73078ee6016dee953488566426cf55fc8011edd0085634d75395f92103cd3e383ec6e12719a6c69515e5559bcbe037d0aa24c187e1e26ce932e22ad7b354ae680800483045022100c99a13fec8eca738ed941d5683342938cf2bb1e339a9938db64589cdd7dd1095022048e1d2b305d919424387456250adf9b4dcb688a8e2d5a3154ad977c8f3525a3901483045022100e2f3a2000c789a46288bd9f51894d8e67cc12227d0909868a52d3bd782b6124e022064c9106a6945cc1f9369f20383f76b83e193b902786f4d9cdd7bfc0299232f5501473044022034753f3fa57d8f8b9fb98324c5e250805cc0662d37912818431d5838e8cbc8a8022016523e5c9a067936a60d0e8e16245576541900f0dd5ae9fada44d33a8677ac9001483045022100d550ad9d5c33e92b0f12e714cbdd18b2522e73886d3e162b57a99523e564142b02201de44c2e4625adf578fb8167030f2e6105a121dec487762c4d5e8531d68032ba01483045022100d03a25a6332ecd3e8486b49afa0cca0c61c895ad6c0556dee0a4a277de3f61a3022051497aa8ce2ad5063ae26d80692d56af44d2a2a87d61e8d46b2a83e2bfda19840100fdc901645521026df77fe41e8ac503ba47cb3a27e12661c5ee9d7f9f185d11c5680c0923356c3e21027279e6b34e38bca7c2b05524eb942beb8e5ef4a1e88cadb366eec66128e3036021027ec0acfd4ae4af03dc9559e0508d98b1790607165d8f95a1d4dcd4da47c05743210285409152cf9031c098a968cde3bcc1e85e4135f0dc8bc022d903a0395a84385221028c561a89624fa81819167ecf6175703c5760fce9a7df6703fa0589050c8f02522102b0db2c66fbad3a46f2b0a617660a66ad72f5391aec659dd4b4de5e45d642e40421031c749a4e732bf871ec985496431b71d85c533690c12a4228143cc290c928078f21031d881aabb972128028fc1b830921f1cc9009a51629c42dd59c7582bfef19a90121036613ff4a2959177ab44913d0a900f9917dab43161b126113d6a098994330330d59ae670350cd00b275532102370a9838e4d15708ad14a104ee5606b36caaaaf739d833e67770ce9fd9b3ec80210257c293086c4d4fe8943deda5f890a37d11bebd140e220faa76258a41d077b4d42103c2660a46aa73078ee6016dee953488566426cf55fc8011edd0085634d75395f92103cd3e383ec6e12719a6c69515e5559bcbe037d0aa24c187e1e26ce932e22ad7b354ae6800000000");
BtcTransaction btcTx = new BtcTransaction(networkParameters, btcRawTx);
int expectedVirtualBytes = 890;

// act
int actualVirtualBytes = calculateBtcTxVirtualSize(btcTx);

// assert
Assertions.assertEquals(expectedVirtualBytes, actualVirtualBytes);
Copy link
Contributor

@julia-zack julia-zack Dec 10, 2025

Choose a reason for hiding this comment

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

very very nice

}
}
61 changes: 58 additions & 3 deletions rskj-core/src/test/java/co/rsk/peg/ReleaseTransactionBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package co.rsk.peg;

import static co.rsk.peg.BridgeUtils.calculateBtcTxVirtualSize;
import static co.rsk.peg.ReleaseTransactionBuilder.BTC_TX_VERSION_1;
import static co.rsk.peg.ReleaseTransactionBuilder.BTC_TX_VERSION_2;
import static org.junit.jupiter.api.Assertions.*;
Expand All @@ -43,6 +44,7 @@
import java.util.List;

import co.rsk.peg.federation.*;
import co.rsk.test.builders.PegoutTransactionBuilder;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest;
import org.junit.jupiter.api.Assertions;
Expand All @@ -60,7 +62,9 @@ class ReleaseTransactionBuilderTest {
private final Coin feePerKb = Coin.MILLICOIN.multiply(2);
private final Federation activeP2shErpFederation = P2shErpFederationBuilder.builder().build();
private final Script activeFederationP2SHScript = activeP2shErpFederation.getP2SHScript();
private final Federation p2shP2wshErpProposedFederation = P2shP2wshErpFederationBuilder.builder().build();
private final List<BtcECKey> signingKeys = BitcoinTestUtils.getBtcEcKeys(20);
private final Federation p2shP2wshErpProposedFederation = P2shP2wshErpFederationBuilder.builder().withMembersBtcPublicKeys(signingKeys).build();
private final Federation activeP2shP2wshErpFederation = P2shP2wshErpFederationBuilder.builder().withMembersBtcPublicKeys(signingKeys).build();
private Wallet wallet;
private ReleaseTransactionBuilder builder;
private Federation federation;
Expand Down Expand Up @@ -354,6 +358,54 @@ void buildMigrationTransaction_withAFederationWithEnoughUTXOs_beforeRSKIP376_sho
assertEquals(BTC_TX_VERSION_1, migrationTransactionUnsigned.getVersion());
}

@Test
void createPegoutTransaction_with210Inputs_with50Outputs_shouldHaveASizeAboveMaximumBtcStandardSize() {
Copy link
Contributor

@julia-zack julia-zack Dec 12, 2025

Choose a reason for hiding this comment

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

should we do these tests for migrations also?

Copy link
Contributor Author

@julianlen julianlen Dec 12, 2025

Choose a reason for hiding this comment

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

As it is right now, a migration tx has only 3 outputs (if there is change). Would you think it makes sense at this stage? Perhaps in the future with multiple utxos?

Copy link
Contributor

@julia-zack julia-zack Dec 12, 2025

Choose a reason for hiding this comment

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

i mean creating it manually, no big deal at all anyways

Copy link
Contributor

@julia-zack julia-zack Dec 12, 2025

Choose a reason for hiding this comment

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

(btw, there's no change in the migration tx)

Copy link
Contributor

@nathanieliov nathanieliov Dec 12, 2025

Choose a reason for hiding this comment

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

The test naming is confusing to me. The name says you are testing the createPegoutTransaction method, but it is a test helper method, so I guess what is being tested is calculateSignedSegwitBtcTxVirtualSize. Then, why have these tests in ReleaseTransactionBuilderTest? According to what I see, these tests are using nothing from it. I would rather move them to BridgeUtilsTest, or better yet, to their own test class. Wdyt?

Copy link
Contributor Author

@julianlen julianlen Dec 12, 2025

Choose a reason for hiding this comment

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

these tests are one step before creating migration transactions with multiple utxos. That is why it is confusing. The test will be createMigrationTransactionWithMultipleUtxos..

In BridgeUtilsTest I am testing calculateBtcTxVirtualSize_withARealBtcTx_shouldReturnCorrectVirtualSize

Copy link
Contributor

@nathanieliov nathanieliov Dec 16, 2025

Choose a reason for hiding this comment

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

The same, let's create a task for the future refactors.

// arrange
int numberOfInputs = 210;

// act
BtcTransaction pegoutTx = createPegoutTransactionWithNInputsAnd50Outputs(activeP2shP2wshErpFederation, signingKeys, numberOfInputs);
int size = calculateBtcTxVirtualSize(pegoutTx);

// assert
assertTrue(size > BtcTransaction.MAX_STANDARD_TX_SIZE);
}

@Test
void createPegoutTransaction_with150Inputs_with50Outputs_shouldHaveASizeBelowMaximumBtcStandardSize() {
// arrange
int numberOfInputs = 150;

// act
BtcTransaction pegoutTx = createPegoutTransactionWithNInputsAnd50Outputs(activeP2shP2wshErpFederation, signingKeys, numberOfInputs);
int size = calculateBtcTxVirtualSize(pegoutTx);

// assert
assertTrue(size < BtcTransaction.MAX_STANDARD_TX_SIZE);
}

private BtcTransaction createPegoutTransactionWithNInputsAnd50Outputs(Federation activeFederation, List<BtcECKey> signingKeys, int numberOfInputs) {
List<UTXO> utxos = getNUtxos(activeFederation.getP2SHScript(), numberOfInputs);

PegoutTransactionBuilder pegoutTransactionBuilder = PegoutTransactionBuilder.builder()
.withNetworkParameters(btcMainNetParams)
.withSignatures(signingKeys)
.withActiveFederation(activeFederation);

byte[] outputScript = activeFederation.getP2SHScript().getProgram();
for (int i = 0; i < utxos.size(); i++) {
pegoutTransactionBuilder = pegoutTransactionBuilder.withInput(BitcoinTestUtils.createHash(i), 0, utxos.get(i).getValue(), outputScript);
}

int numberOfOutputs = 49; // The 50th transaction output will be the change output
for (int i = 0; i < numberOfOutputs; i++) {
Address recipient = BitcoinTestUtils.createP2PKHAddress(btcMainNetParams, "address" + i);
pegoutTransactionBuilder = pegoutTransactionBuilder.withOutput(Coin.SATOSHI, recipient);
}

return pegoutTransactionBuilder.build();
}

@Test
void buildMigrationTransaction_withAFederationWithEnoughUTXOs_afterRSKIP376_shouldReturnACorrectMigrationTx_withVersion2() {
// Arrange
Expand Down Expand Up @@ -567,8 +619,7 @@ void build_pegout_tx_from_non_standard_erp_federation() {
assertEquals(ReleaseTransactionBuilder.Response.SUCCESS, result.getResponseCode());
}

private static List<UTXO> getUtxos(Script outputScript) {
int numberOfUtxos = 2;
private static List<UTXO> getNUtxos(Script outputScript, int numberOfUtxos) {
Coin value = Coin.COIN;
ArrayList<UTXO> utxos = new ArrayList<>(numberOfUtxos);
for (int i = 0; i < numberOfUtxos; i++) {
Expand All @@ -585,6 +636,10 @@ private static List<UTXO> getUtxos(Script outputScript) {
return utxos;
}

private static List<UTXO> getUtxos(Script outputScript) {
return getNUtxos(outputScript, 2);
}

@Test
void getters() {
Assertions.assertSame(wallet, builder.getWallet());
Expand Down
4 changes: 2 additions & 2 deletions rskj-core/src/test/java/co/rsk/test/builders/PegoutTransactionBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ public PegoutTransactionBuilder withActiveFederation(Federation activeFederation
return this;
}

public PegoutTransactionBuilder withInput(Sha256Hash spendTxHash, int outputIndex, Coin value) {
public PegoutTransactionBuilder withInput(Sha256Hash spendTxHash, int outputIndex, Coin value, byte[] scriptSig) {
TransactionInput input = new TransactionInput(
networkParameters,
null,
new byte[]{},
scriptSig,
new TransactionOutPoint(networkParameters, outputIndex, spendTxHash),
value
);
Expand Down