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

Jump to bottom

Erase the constant REGTEST_FEDERATION_PRIVATE_KEYS from the tests #2930

Open

julianlen wants to merge 12 commits into bridge-refactors-integration from erase-genesisfed-pKeys

Contributor

julianlen commented Jan 13, 2025 •

edited

Loading

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:

github-actions bot commented Jan 13, 2025 •

edited

Loading

Dependency Review

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

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

github-advanced-security bot found potential problems

View reviewed changes

rskj-core/src/test/java/co/rsk/peg/PegUtilsLegacyTest.java

-                      Address federationAddress = genesisFederation.getAddress();
-                      wallet.addWatchedAddress(federationAddress, genesisFederation.getCreationTime().toEpochMilli());
+                      Federation federation = getErpFederationWithPrivKeys(networkParameters, fedKeys);
+                      Wallet wallet = new BridgeBtcWallet(btcContext, Collections.singletonList(federation));

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [PegUtilsLegacy.isValidPegInTx](1) should be avoided because it has been deprecated.

rskj-core/src/test/java/co/rsk/peg/PegUtilsLegacyTest.java

    
                      Wallet federationWallet = new BridgeBtcWallet(btcContext, Collections.singletonList(genesisFederation));

                      assertTrue(isValidPegInTx(tx, genesisFederation, federationWallet, bridgeConstantsMainnet, activations));

                      Wallet federationWallet = new BridgeBtcWallet(btcContext, Collections.singletonList(federation));

                      assertTrue(isValidPegInTx(tx, federation, federationWallet, bridgeConstantsMainnet, activations));

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [PegUtilsLegacy.isValidPegInTx](1) should be avoided because it has been deprecated.

rskj-core/src/test/java/co/rsk/peg/PegUtilsLegacyTest.java

    
                      Wallet federationWallet = new BridgeBtcWallet(btcContext, Collections.singletonList(genesisFederation));

                      assertFalse(isValidPegInTx(tx, genesisFederation, federationWallet, bridgeConstantsMainnet, activations));

                      Wallet federationWallet = new BridgeBtcWallet(btcContext, Collections.singletonList(federation));

                      assertFalse(isValidPegInTx(tx, federation, federationWallet, bridgeConstantsMainnet, activations));

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [PegUtilsLegacy.isValidPegInTx](1) should be avoided because it has been deprecated.

rskj-core/src/test/java/co/rsk/peg/PegUtilsLegacyTest.java

    
                      Wallet federationWallet = new BridgeBtcWallet(btcContext, Collections.singletonList(genesisFederation));

                      assertTrue(isValidPegInTx(tx, genesisFederation, federationWallet, bridgeConstantsMainnet, activations));

                      Wallet federationWallet = new BridgeBtcWallet(btcContext, Collections.singletonList(federation));

                      assertTrue(isValidPegInTx(tx, federation, federationWallet, bridgeConstantsMainnet, activations));

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [PegUtilsLegacy.isValidPegInTx](1) should be avoided because it has been deprecated.

rskj-core/src/test/java/co/rsk/peg/PegUtilsLegacyTest.java


		assertTrue(scriptCorrectlySpendsTx(tx, 0, genesisFederation.getP2SHScript()));
		assertTrue(scriptCorrectlySpendsTx(tx, 0, federation.getP2SHScript()));

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [PegUtilsLegacy.scriptCorrectlySpendsTx](1) should be avoided because it has been deprecated.

rskj-core/src/test/java/co/rsk/peg/PegUtilsLegacyTest.java

@@ @@ -2289,7 +2285,7 @@ @@
                           .build();
                       tx.getInput(0).setScriptSig(invalidScript);
-                      assertFalse(scriptCorrectlySpendsTx(tx, 0, genesisFederation.getP2SHScript()));
+                      assertFalse(scriptCorrectlySpendsTx(tx, 0, federation.getP2SHScript()));

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [PegUtilsLegacy.scriptCorrectlySpendsTx](1) should be avoided because it has been deprecated.

julianlen marked this pull request as ready for review

January 13, 2025 19:07

julianlen requested a review from a team as a code owner

January 13, 2025 19:07

Contributor Author

julianlen commented Jan 13, 2025

The tests in https://github.com/rsksmart/rskj/blob/398faa59ef120c989d61cacbd4492969bbe5bf59/rskj-core/src/test/java/co/rsk/peg/RskForksBridgeTest.java use REGTEST_FEDERATION_PRIVATE_KEYS. However, it's not straightforward to erase it since it uses the testing class World which has embedded the constants for the bridge

julianlen force-pushed the erase-genesisfed-pKeys branch from 343f19d to aa90966 Compare

January 14, 2025 14:36

julianlen changed the base branch from master to bridge-refactors-integration

January 14, 2025 14:36

julianlen added 8 commits

January 14, 2025 15:47


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from PegUt…

951e36a

…ilsLegacyTest


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from Bridg…

9cb1ae7

…eSupportIT


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from TxVal…

e1a0e25

…idatorIntrinsicGasLimitValidatorTest


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from Bridg…

13442c0

…eIT (ex BridgeTestIntegration.java)


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from Bridg…

5a723eb

…eUtilTest


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from Bridg…

ab6c2df

…eSupportAddSignatureTest


          feat: deleted the constant REGTEST_FEDERATION_PRIVATE_KEYS from Bridg…

3f208af

…eCostsTest


          fix: fixed various suggestions by Sonarqube

7c135be

julianlen force-pushed the erase-genesisfed-pKeys branch from 8ba7ec8 to 7c135be Compare

January 14, 2025 18:47

julia-zack reviewed

View reviewed changes

rskj-core/src/main/java/org/ethereum/config/Constants.java Outdated

@@ @@ -304,7 +304,7 @@ public static Constants regtest() { @@
                       );
                   }
-                  public static Constants regtestWithFederation(List<BtcECKey> genesisFederationPublicKeys) {
+                  public static Constants regtestWithFederation(List<BtcECKey> federation) {

Contributor

julia-zack Jan 14, 2025

Suggested change

      
                public static Constants regtestWithFederation(List<BtcECKey> federation) {
          
                public static Constants regtestWithFederation(List<BtcECKey> federationPublicKeys) {

Contributor Author

julianlen Jan 14, 2025

done!

julia-zack reviewed

View reviewed changes

.../src/test/java/co/rsk/net/handler/txvalidator/TxValidatorIntrinsicGasLimitValidatorTest.java Outdated

-                      BtcECKey.fromPrivate(Hex.decode("45c5b07fc1a6f58892615b7c31dca6c96db58c4bbc538a6b8a22999aaa860c32")),
-                      BtcECKey.fromPrivate(Hex.decode("505334c7745df2fc61486dffb900784505776a898377172ffa77384892749179")),
-                      BtcECKey.fromPrivate(Hex.decode("bed0af2ce8aa8cb2bc3f9416c9d518fdee15d1ff15b8ded28376fcb23db6db69"))
+                  private static final List<BtcECKey> retiringFedKeys = BitcoinTestUtils.getBtcEcKeysFromSeeds(

Contributor

julia-zack Jan 14, 2025

why retiring fed?

Contributor Author

julianlen Jan 14, 2025

It was a copy paste, my bad! Fixed!

julia-zack reviewed

View reviewed changes

rskj-core/src/test/java/co/rsk/peg/BridgeCostsTest.java Outdated

-                      BtcECKey.fromPrivate(Hex.decode("45c5b07fc1a6f58892615b7c31dca6c96db58c4bbc538a6b8a22999aaa860c32")),
-                      BtcECKey.fromPrivate(Hex.decode("505334c7745df2fc61486dffb900784505776a898377172ffa77384892749179")),
-                      BtcECKey.fromPrivate(Hex.decode("bed0af2ce8aa8cb2bc3f9416c9d518fdee15d1ff15b8ded28376fcb23db6db69"))
+                  private static final List<BtcECKey> fedECKeys = BitcoinTestUtils.getBtcEcKeysFromSeeds(

Contributor

julia-zack Jan 14, 2025

Suggested change

      
                private static final List<BtcECKey> fedECKeys = BitcoinTestUtils.getBtcEcKeysFromSeeds(
          
                private static final List<BtcECKey> fedBtcECKeys = BitcoinTestUtils.getBtcEcKeysFromSeeds(

careful, ECKeys is for rsk and mst keys.

Contributor Author

julianlen Jan 14, 2025

how's that?

Contributor Author

julianlen Jan 14, 2025

Just in case, it is fixed already!

julianlen added 4 commits

January 14, 2025 18:30


          fix: changed the parameter federation to federationPublicKeys in the …

2251d3b

…regtestWithFederation method


          fix: changed the variable retiringFedKeys which was wrong for fedKeys

3fbd991


          fix: replace the variable fedECKeys by fedBtcECKeys in the BridgeCost…

b91418d

…sTest


          fix: unified the names of the keys to fedBtcECKeys

1ab8a90

sonarqubecloud bot commented Jan 14, 2025

Quality Gate passed

Issues
132 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

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

Labels

None yet