Save the entire svp fund tx instead of only its hash #2731

julia-zack · 2024-09-16T23:53:10Z

Background
To create the svp spend transaction, we will need to have the entire svp fund transaction signed, and not only its hash.

Requirements
-Modify all the places where the svp fund transaction hash signed was being considered to now consider the entire svp fund transaction signed.
-Create new serialize/deserializeBtcTransaction methods in BridgeSerializationUtils to save the serialized tx.

Reorder rskip

Add tests Refactor Rename variable

Change name

Add key to storage index Remove declaration of svp fund tx hash since it is not being used yet. Remove _KEY suffix

Remove _KEY suffix. Save proposed federation the same way as pending one

Add proposedFederationIsSet logic Getting rid of proposedFederationIsSet logic. Add isProposedFederationSet logic. Add rskip419 check in saveProposedFederation method Save null version when proposed federation is null Remove unnecessary private method Refactor Removes null activations from tests

Make variable final Improve test name Use all activations instead of lovell Improve null handling

Add get proposed federation tests Minor fix after rebase Improve comment Minor refactor Minor refactor Improve comment Add test case Throw exception when there is no storage version for non-null proposed federation Add test cases Add log. Add test case and refactor Remove unused import

…sh methods and tests Rebases Reorders and renames tests Rebases Appends SVP prefix to FUND_TX_HASH_UNSIGNED and refactors tests Rebases Moves repeated arrange code to setup. Using Optional. Rebases Removes _ from the key Using arrowhead631 Rebases Rebases Renames saveFundTransactionUnsignedHash to saveSvpFundTransactionUnsignedHash Renames FundTransactionUnsignedHash instance fields

Remove unnecessary private method Refactor

Add get proposed federation tests Minor fix after rebase Improve comment Minor refactor Add test case

…d federation Add test cases Add log. Add test case and refactor Remove unused import

…sh methods and tests Rebases Moves repeated arrange code to setup. Using Optional. Rebases

Adds test to assert empty is returned when hash hasn't been set or saved Renames svp fields to match the RSKIP419 description. Adds more tests. Using standard. Renames test

Improve variable name Add comment Minor refactors Improve comments. Minor refactors Improve comments Refactor Add tests for commitFederationAccordingToActivations Remove unnecessary semicolon Refactor to improve testing and readability Add test cases Remove some tests. Make bridge event logger not a mock. Make some methods private instead of protected. Fix log message Improve test name Change test name Move reused method to utility class. Use real block instead of a mock Make methods to be private instead of protected

Add method to utils

Fix sonar complains Add missing import Move method to test class Remove unused imports

…y to bridge storage index key

… to bridge storage provider Remove svp fund tx tests from federation storage provider tests, and adds them to bridge storage provider tests Get rid of sonar complains Add get methods to bridge Put save, set and get tests in same nested class

Add tests. Add missing tests for minimum pegout tx value Modify test

Remove unused import Remove mistaken reason Minor refactor Make flyover rs builder receive a keccak256 hash instead of sha256 one Refactors

…o RedeemScriptCreationException

Rename

… creating pegouts

Refactor and add test case Add test case Remove mistaken comment Add comment Modify comment

julia-zack · 2024-09-17T00:05:38Z

rskj-core/src/main/java/co/rsk/peg/BridgeSerializationUtils.java

-    public static byte[] serializeSet(SortedSet<Sha256Hash> set) {
-        int nhashes = set.size();
-
-        byte[][] bytes = new byte[nhashes][];
-        int n = 0;
-
-        for (Sha256Hash hash : set) {
-            bytes[n++] = RLP.encodeElement(hash.getBytes());
-        }
-
-        return RLP.encodeList(bytes);
-    }
-
-    public static SortedSet<Sha256Hash> deserializeSet(byte[] data) {
-        SortedSet<Sha256Hash> set = new TreeSet<>();
-
-        if (data == null || data.length == 0) {
-            return set;
-        }
-
-        RLPList rlpList = (RLPList)RLP.decode2(data).get(0);
-
-        int nhashes = rlpList.size();
-
-        for (int k = 0; k < nhashes; k++) {
-            set.add(Sha256Hash.wrap(rlpList.get(k).getRLPData()));
-        }
-
-        return set;
-    }
-


this has been removed since it is not being used anywhere

julia-zack · 2024-09-17T12:17:45Z

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

+            BtcTransaction prevTx = new BtcTransaction(mainnetBtcParams);
+            Address address = BitcoinTestUtils.createP2PKHAddress(mainnetBtcParams, "address");
+            prevTx.addOutput(Coin.FIFTY_COINS, address);
+            svpFundTx.addInput(prevTx.getOutput(0));


just to make the svpFundTx more complete, and different from the anotherSvpFundTx

… avoid passing boolean

apancorb

LGTM. minor comment

rskj-core/src/main/java/co/rsk/peg/BridgeStorageIndexKey.java

…blank lines to improve readability

…thods to deserialize btc txs with and without inputs.

… avoid passing boolean

sonarqubecloud · 2024-09-18T18:41:05Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

julia-zack and others added 30 commits September 12, 2024 13:08

Add rskip419 config to lovell

90585fb

Reorder rskip

Add validation period duration to federation constants

557052b

Add tests Refactor Rename variable

Add proposedFederation and validationFundTxHashUnsigned

9bc5a84

Change name

Add keys to storage index

8c08105

Add key to storage index Remove declaration of svp fund tx hash since it is not being used yet. Remove _KEY suffix

Add set and save methods for the proposed federation

ab43f46

Remove _KEY suffix. Save proposed federation the same way as pending one

Add tests for saveProposedFederation method

b0a39f5

Make variable final Improve test name Use all activations instead of lovell Improve null handling

Add format version assertion to tests

54a80cd

Add set and save methods for the proposed federation

f67cbb4

Save null version when proposed federation is null

105391e

Remove unnecessary private method Refactor

Add getProposedFederation method

e73cc3b

Add get proposed federation tests Minor fix after rebase Improve comment Minor refactor Add test case

Throw exception when there is no storage version for non-null propose…

fe43703

…d federation Add test cases Add log. Add test case and refactor Remove unused import

Adds setFundTransactionUnsignedHash and saveFundTransactionUnsignedHa…

ebd7d84

…sh methods and tests Rebases Moves repeated arrange code to setup. Using Optional. Rebases

Adds getSvpFundTransactionUnsignedHash and tests

22105e7

Adds test to assert empty is returned when hash hasn't been set or saved Renames svp fields to match the RSKIP419 description. Adds more tests. Using standard. Renames test

Add tests to existing nested class

feb9b56

Add method to utils

Comment broken test that be addressed later

5b9d939

Fix sonar complains Add missing import Move method to test class Remove unused imports

Move svp fund tx hash unsigned entry from federation storage index ke…

683a9b4

…y to bridge storage index key

Define spendable value directly in BridgeConstants

35a3618

Add tests. Add missing tests for minimum pegout tx value Modify test

Add getProposedFederation method in FederationSupport

f7935fd

Create flyover redeem script builder

7e92781

Remove unused import Remove mistaken reason Minor refactor Make flyover rs builder receive a keccak256 hash instead of sha256 one Refactors

Delete FlyoverRedeemScriptCreationException and move flyover reason t…

c9d139a

…o RedeemScriptCreationException

Add proposed federation dummy flyover prefix to bridge constants

43eb320

Rename

Create new settleReleaseRequest method to perform common actions when…

f6510c2

… creating pegouts

Add svp fund tx hash signed storage index key

5ab3636

Add set and save methods

bef3d18

Add getSvpFundTxHashSigned method

cf3b35c

Refactor and add test case Add test case Remove mistaken comment Add comment Modify comment

julia-zack commented Sep 17, 2024

View reviewed changes

julia-zack marked this pull request as ready for review September 17, 2024 12:24

julia-zack requested a review from a team as a code owner September 17, 2024 12:24

julia-zack added 2 commits September 17, 2024 16:06

Create new methods to handle btc tx deserialization without inputs to…

ed3de69

… avoid passing boolean

Remove boolean from deserialization in provider

f03073e

julia-zack force-pushed the save_fund_tx_signed_instead_of_hash branch from cc7bea6 to 45e7d97 Compare September 17, 2024 19:15

julia-zack changed the base branch from feature/powpeg_validation_protocol-phase2 to feat/refactor_bridge_serialization_utils September 17, 2024 19:15

apancorb approved these changes Sep 18, 2024

View reviewed changes

rskj-core/src/main/java/co/rsk/peg/BridgeStorageIndexKey.java Show resolved Hide resolved

julia-zack added 6 commits September 18, 2024 09:40

Rename fundTx to prevTx to avoid svp confusion. Fix indentation. Add …

29227eb

…blank lines to improve readability

Declare network params from bridge constants to fix sonar complains

cd7a4a0

Fix indentation

a15016f

Fix indentation

fac8356

Improve variables naming

c22b072

Improve variables naming

e468239

julia-zack changed the title ~~When signed, save the svp fund tx instead of its hash~~ Save the entire svp fund tx instead of only its hash Sep 18, 2024

julia-zack force-pushed the save_fund_tx_signed_instead_of_hash branch from 45e7d97 to 14712e3 Compare September 18, 2024 15:43

julia-zack added 6 commits September 18, 2024 15:05

Keep bitcoin impl for deserializeBtcTransaction method, but create me…

5a1e6c4

…thods to deserialize btc txs with and without inputs.

When signed, save the svp fund tx instead of its hash

96116a8

Correct typo and comments

becc121

Create new methods to handle btc tx deserialization without inputs to…

7a0a1d1

… avoid passing boolean

Fix typo

8beace0

Use new deserialization method for btc tx with inputs

45b96c4

julia-zack force-pushed the save_fund_tx_signed_instead_of_hash branch from 14712e3 to 45b96c4 Compare September 18, 2024 18:22

marcos-iov force-pushed the feat/refactor_bridge_serialization_utils branch from 5a1e6c4 to 44c4644 Compare September 18, 2024 19:02

Base automatically changed from feat/refactor_bridge_serialization_utils to feature/powpeg_validation_protocol-phase2 September 18, 2024 19:29

marcos-iov force-pushed the feature/powpeg_validation_protocol-phase2 branch from 98ed61b to 4835a42 Compare September 18, 2024 20:13

julia-zack changed the base branch from feature/powpeg_validation_protocol-phase2 to feat/refactor_bridge_serialization_utils_recovery September 19, 2024 00:31

julia-zack closed this Sep 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save the entire svp fund tx instead of only its hash #2731

Save the entire svp fund tx instead of only its hash #2731

julia-zack commented Sep 16, 2024

julia-zack Sep 17, 2024 •

edited

Loading

julia-zack Sep 17, 2024 •

edited

Loading

apancorb left a comment

sonarqubecloud bot commented Sep 18, 2024

Save the entire svp fund tx instead of only its hash #2731

Save the entire svp fund tx instead of only its hash #2731

Conversation

julia-zack commented Sep 16, 2024

julia-zack Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

julia-zack Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

apancorb left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Sep 18, 2024

Quality Gate passed

julia-zack Sep 17, 2024 •

edited

Loading

julia-zack Sep 17, 2024 •

edited

Loading