Bridge Constants Refactor Integration #2309
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
pipeline:run |
wilmerrootstock
force-pushed
the
bridge-constants-refactor-integration
branch
from
036c773 to
6849f76
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -743,7 +743,7 @@ | |||
| BridgeStorageProvider provider0 = new BridgeStorageProvider(track, PrecompiledContracts.BRIDGE_ADDR, bridgeConstants, activationsBeforeForks); | |||
|
|
|||
| provider0.getReleaseRequestQueue().add(new BtcECKey().toAddress(btcParams), Coin.COIN); | |||
| provider0.getNewFederationBtcUTXOs().add(new UTXO(PegTestUtils.createHash(), 1, Coin.COIN.add(Coin.valueOf(100)), 0, false, ScriptBuilder.createOutputScript(federation.getAddress()))); | |||
| provider0.getNewFederationBtcUTXOs().add(new UTXO(PegTestUtils.createHash(), 1, Coin.COIN.add(Coin.valueOf(100)), 0, false, ScriptBuilder.createOutputScript(genesisFederation.getAddress()))); | |||
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| FederationArgs genesisFedArgs = new FederationArgs(FederationTestUtils.getFederationMembersWithKeys( | ||
| Stream.iterate(1, i -> i + 1) | ||
| .limit(6) | ||
| .map(i -> BtcECKey.fromPrivate(BigInteger.valueOf((i) * 100))) |
Check warning
Code scanning / CodeQL
Result of multiplication cast to wider type Warning test
wilmerrootstock
force-pushed
the
bridge-constants-refactor-integration
branch
from
6849f76 to
eed3809
Compare
marcos-iov
force-pushed
the
bridge-constants-refactor-integration
branch
from
eed3809 to
7b8a31b
Compare
|
pipeline:run |
nathanieliov
reviewed
May 9, 2024
rskj-core/src/main/java/co/rsk/peg/constants/BridgeTestNetConstants.java
Outdated
Show resolved
Hide resolved
marcos-iov
force-pushed
the
bridge-constants-refactor-integration
branch
from
30bf8ed to
e186005
Compare
|
pipeline:run |
1 similar comment
|
pipeline:run |
marcos-iov
force-pushed
the
bridge-constants-refactor-integration
branch
from
0fade16 to
9b13500
Compare
|
pipeline:run |
marcos-iov
force-pushed
the
bridge-constants-refactor-integration
branch
from
9b13500 to
d0f57a9
Compare
- Moving BridgeConstants class and all classes (regtest, testnet, mainnet, DevNet) related to co.rsk.peg.constants package. - Moving BridgeConstantsTest class to the same package inside tests section.
- optimizing imports.
- Converting list as an unmodifiableList to fix issue reported by SonarQube for List that are declared as public static since making a mutable field, such as an array, final will keep the variable from being reassigned, but doing so has no effect on the mutability of the internal state of the array.
- Moving BridgeConstants class and all classes (regtest, testnet, mainnet, DevNet) related to co.rsk.peg.constants package. - Moving BridgeConstantsTest class to the same package inside tests section.
- Removing class variable, injection in the constructor and its implementation. - Modifying isFromGenesisFederation method signature to add List<BtcECKey> genesisFederationPublicKeys as an in parameter.
- Adding changes to the tests to adapt it to new way to get Genesis Federation. - Creating a new constant called GENESIS_FEDERATION_CREATION_BLOCK_NUMBER in FederationTestUtils.java and this same variable/constant in FederationSupport#getGenesisFederation - Improving the readability in the statement lambda in BridgeUtils#isFromGenesisFederation - New test to make sure BrigdeConstants has the correct GenesisFederationCreationTime according to the network. - Improving Utils classes: FederationTestUtils: adding final keyword at level class to avoid this class could be inherited and to improve the class performance. adding a private constructor to avoid this utility class could be instantiated BridgeUtils: adding final keyword at level class to avoid this class could be inherited and to improve the class performance.
- Reformat code in a code block in BridgeSupportAddSignatureTest.java - Reusing mainnet in classes where it already exists. Replacing Regtest.
- Addressing review comment: Use meaningful variable names. Improving Federation variables
- Cleaning up fix for the unit test BridgeSupportTestIntegration#getFederationMethods_genesis.
- Fixing variable names issues: 1.genesisFederation turns into activeFederation since it is used by a When clause like an activeFederation. 2. genesisFederation2 turns into genesisFederation.
1. Indentation on BridgeSupportAddSignatureTest#addSignature_fedPubKey_belongs_to_active_federation 2. On BridgeUtils#createPegOutTx, variable genesisFederation turns into federation since it could be different types of Federations 3. On FederationSupportTest was improved the whenNewFederationIsNullThenActiveFederationIsGenesisFederation Unit Test. 4. GENESIS_FEDERATION_CREATION_BLOCK_NUMBER was removed as class level constant and was included in the only one method it is being used. 5. On P2shErpFederationTest#getBtcPublicKeys() was removed the creation of genesisFederation and use getGenesisFederationPublicKeys() directly from BridgeConstants
Replacing BridgeRegTestConstants values with BridgeMainNetConstants values.
The FederationMember class contains a KeyType enum. This enum has a Value field that must be declared with the final keyword.
releaseBtc method is printing a message showing the wrong unit. Change the log message to print Weis as the unit. Get the value in weis from the rsk transaction before converting it to satoshis with rskTx.getValue().
- Changed RBTC to weis in the log message.
There are 2 bridge constants related to the minimum pegout value. legacyMinimumPegoutTxValueInSatoshis and minimumPegoutTxValueInSatoshis The name is not precise, because the data type is actually a Coin object, so it’s not an amount and should not have a unit. For this reason, we renamed both constants to legacyMinimumPegoutTxValue and minimumPegoutTxValue respectively.
marcos-iov
force-pushed
the
bridge-constants-refactor-integration
branch
from
d0f57a9 to
36ca811
Compare
|
|
pipeline:run |
marcos-iov
approved these changes
May 17, 2024
|
pipeline:run |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
All these changes are part of a general Bridge Constants Refactor:
fed:bridge-constants-refactor-integration
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist: