Skip to content
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

Refactor addSignature as previous step to create addSvpSpendTxSignature method #2788

Merged
merged 16 commits into from
Oct 14, 2024
Merged

Refactor addSignature as previous step to create addSvpSpendTxSignature method #2788

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c6c6a5f
Refactor addSignature as previous step to create addSvpSpendTxSignatu…
julia-zack Oct 3, 2024
d0eb08b
Remove addSvpSpendTxSignature method that would be part of another pr
julia-zack Oct 3, 2024
e1b8704
Remove optional logic to verify signatures
julia-zack Oct 3, 2024
249cbd0
Catch specific SignatureException
julia-zack Oct 3, 2024
ccfdcde
Make getDecodedSignatures throw signature exception to handle just th…
julia-zack Oct 3, 2024
f14bece
Avoid using isPresent / get and make code more functional. Align java…
julia-zack Oct 4, 2024
31a9c69
Change boolean method name to be affirmative. Change method responsib…
julia-zack Oct 8, 2024
074d749
Use getScriptSigWithSignature instead of updateScriptWithSignature
julia-zack Oct 8, 2024
5a186d9
Rename boolean method. Replace rskTxHashSerialized argument for the k…
julia-zack Oct 9, 2024
afccbd5
Add tests for new utility methods. Rename method to be more specific
julia-zack Oct 9, 2024
5aa566c
Add test for when method should throw
julia-zack Oct 9, 2024
e7a67b1
Add log when catching exception to avoid sonar warning
julia-zack Oct 9, 2024
361b013
Add logging error message when logging caught exception. Throw a dedi…
julia-zack Oct 10, 2024
2fabadb
Remove unused import
julia-zack Oct 10, 2024
c6af3f4
Use already existing method to extract redeem script. Remove unnecess…
julia-zack Oct 11, 2024
5c43d68
Throw exception when trying to serialize a null value. Move tests to …
julia-zack Oct 14, 2024
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
13 changes: 6 additions & 7 deletions rskj-core/src/main/java/co/rsk/peg/Bridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package co.rsk.peg;

import static co.rsk.peg.BridgeSerializationUtils.deserializeRskTxHash;
import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP417;

import co.rsk.bitcoinj.core.*;
Expand Down Expand Up @@ -617,15 +618,13 @@ public void addSignature(Object[] args) throws VMException {
}
signatures.add(signatureByteArray);
}
byte[] rskTxHash = (byte[]) args[2];
if (rskTxHash.length!=32) {
throw new BridgeIllegalArgumentException("Invalid rsk tx hash " + Bytes.of(rskTxHash));
}
byte[] rskTxHashSerialized = (byte[]) args[2];
try {
Keccak256 rskTxHash = deserializeRskTxHash(rskTxHashSerialized);
bridgeSupport.addSignature(federatorPublicKey, signatures, rskTxHash);
} catch (BridgeIllegalArgumentException e) {
throw e;
} catch (Exception e) {
} catch (IllegalArgumentException e) {
throw new BridgeIllegalArgumentException("Invalid rsk tx hash " + Bytes.of(rskTxHashSerialized));
} catch (IOException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change a good idea? I think all Bridge methods are capturing generic exceptions

Copy link
Contributor

Choose a reason for hiding this comment

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

There might be unchecked exception thrown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

logger.warn("Exception in addSignature", e);
throw new VMException("Exception in addSignature", e);
}
Expand Down
12 changes: 6 additions & 6 deletions rskj-core/src/main/java/co/rsk/peg/BridgeSerializationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ private static byte[] serializeRskTxHash(Keccak256 rskTxHash) {
return RLP.encodeElement(rskTxHash.getBytes());
}

private static Keccak256 deserializeRskTxHashFromHashData(byte[] hashBytes) {
return new Keccak256(hashBytes);
public static Keccak256 deserializeRskTxHash(byte[] rskTxHashSerialized) {
return new Keccak256(rskTxHashSerialized);
}

public static byte[] serializeBtcTransaction(BtcTransaction btcTransaction) {
Expand Down Expand Up @@ -186,7 +186,7 @@ private static Map.Entry<Keccak256, BtcTransaction> deserializeRskTxWaitingForSi

RLPElement rskTxHashRLPElement = rlpList.get(index * 2);
byte[] rskTxHashData = rskTxHashRLPElement.getRLPData();
Keccak256 rskTxHash = deserializeRskTxHashFromHashData(rskTxHashData);
Keccak256 rskTxHash = deserializeRskTxHash(rskTxHashData);

RLPElement btcTxRLPElement = rlpList.get(index * 2 + 1);
byte[] btcRawTx = btcTxRLPElement.getRLPData();
Expand Down Expand Up @@ -667,7 +667,7 @@ private static List<ReleaseRequestQueue.Entry> deserializeReleaseRequestQueueWit
Address address = new Address(networkParameters, addressBytes);
long amount = BigIntegers.fromUnsignedByteArray(rlpList.get(k * 3 + 1).getRLPData()).longValue();

Keccak256 txHash = deserializeRskTxHashFromHashData(rlpList.get(k * 3 + 2).getRLPData());
Keccak256 txHash = deserializeRskTxHash(rlpList.get(k * 3 + 2).getRLPData());

entries.add(new ReleaseRequestQueue.Entry(address, Coin.valueOf(amount), txHash));
}
Expand Down Expand Up @@ -760,7 +760,7 @@ private static PegoutsWaitingForConfirmations deserializePegoutWaitingForConfirm
BtcTransaction tx = new BtcTransaction(networkParameters, txPayload);

long height = BigIntegers.fromUnsignedByteArray(rlpList.get(k * 3 + 1).getRLPData()).longValue();
Keccak256 rskTxHash = deserializeRskTxHashFromHashData(rlpList.get(k * 3 + 2).getRLPData());
Keccak256 rskTxHash = deserializeRskTxHash(rlpList.get(k * 3 + 2).getRLPData());

entries.add(new PegoutsWaitingForConfirmations.Entry(tx, height, rskTxHash));
}
Expand Down Expand Up @@ -849,7 +849,7 @@ public static FlyoverFederationInformation deserializeFlyoverFederationInformati
if (rlpList.size() != 2) {
throw new RuntimeException(String.format("Invalid serialized Fast Bridge Federation: expected 2 value but got %d", rlpList.size()));
}
Keccak256 derivationHash = deserializeRskTxHashFromHashData(rlpList.get(0).getRLPData());
Keccak256 derivationHash = deserializeRskTxHash(rlpList.get(0).getRLPData());
byte[] federationP2SH = rlpList.get(1).getRLPData();

return new FlyoverFederationInformation(derivationHash, federationP2SH, flyoverScriptHash);
Expand Down
Loading