Skip to content

Conversation

@julianlen
Copy link
Contributor

Description

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@julianlen julianlen requested a review from a team as a code owner December 5, 2025 20:30
@julianlen julianlen requested review from Copilot and removed request for a team December 5, 2025 20:31
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds tests to verify the maximum number of inputs allowed in Bitcoin pegout transactions while staying within the Bitcoin standard transaction size limit. The changes include updates to the PegoutTransactionBuilder to accept custom script signatures, a new utility method for calculating SegWit transaction virtual size, and two test cases that verify transaction sizes with different input counts.

  • Added calculateSignedSegwitBtcTxVirtualSize utility method for calculating transaction virtual sizes
  • Updated PegoutTransactionBuilder.withInput to accept a script parameter instead of hardcoding an empty byte array
  • Added tests verifying that 150 inputs stay below the limit while 210 inputs exceed it

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rskj-core/src/main/java/co/rsk/peg/BridgeUtils.java Extracted virtual byte calculation into reusable methods and added public method for calculating SegWit transaction virtual size
rskj-core/src/test/java/co/rsk/peg/BridgeUtilsTest.java Added test case to verify virtual size calculation using a real SegWit transaction
rskj-core/src/test/java/co/rsk/peg/ReleaseTransactionBuilderTest.java Added tests verifying transaction size limits with varying input counts and refactored UTXO generation helper
rskj-core/src/test/java/co/rsk/test/builders/PegoutTransactionBuilder.java Modified withInput method to accept script parameter for more flexible test setup
rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java Updated test calls to match new withInput signature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2025

@julianlen
Copy link
Contributor Author

As a comment, the method calculateSignedSegwitBtcTxVirtualSize https://github.com/rsksmart/rskj/pull/3405/files#diff-a33646b11694d377aa69060bbfd25b2800f9e3f72e5ecfbbb3b1b4ab16c4da2fR674 is in BridgeUtils.java as production code. Although the tests only use this method.

// act
int actualVirtualBytes = calculateSignedSegwitBtcTxVirtualSize(btcTx);

//assert
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//assert
// assert

return baseSize + signingSize;
}

public static int calculateSignedSegwitBtcTxVirtualSize(BtcTransaction btcTx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public static int calculateSignedSegwitBtcTxVirtualSize(BtcTransaction btcTx) {
public static int calculateBtcTxVirtualSize(BtcTransaction btcTx) {

you are not using anywhere the segwit nor the signed part right?

int actualVirtualBytes = calculateSignedSegwitBtcTxVirtualSize(btcTx);

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

Choose a reason for hiding this comment

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

very very nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants