Skip to content

Commit

Permalink
fix(peg): svp spend tx waiting for signatures double serialization
Browse files Browse the repository at this point in the history
  • Loading branch information
apancorb committed Nov 15, 2024
1 parent 3858201 commit dcd8a9e
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ private static BtcTransaction deserializeBtcTransactionFromRawTx(

public static byte[] serializeRskTxWaitingForSignatures(
Map.Entry<Keccak256, BtcTransaction> rskTxWaitingForSignaturesEntry) {
if (rskTxWaitingForSignaturesEntry == null) {
return RLP.encodedEmptyList();
}

byte[][] serializedRskTxWaitingForSignaturesEntry =
serializeRskTxWaitingForSignaturesEntry(rskTxWaitingForSignaturesEntry);
return RLP.encodeList(serializedRskTxWaitingForSignaturesEntry);
Expand Down Expand Up @@ -154,12 +158,15 @@ private static byte[][] serializeRskTxWaitingForSignaturesEntry(

public static Map.Entry<Keccak256, BtcTransaction> deserializeRskTxWaitingForSignatures(
byte[] data, NetworkParameters networkParameters) {

if (data == null || data.length == 0) {
return null;
}

RLPList rlpList = (RLPList) RLP.decode2(data).get(0);
if (rlpList.size() == 0) {
return null;
}

return deserializeRskTxWaitingForSignaturesEntry(rlpList, 0, networkParameters);
}

Expand Down
5 changes: 3 additions & 2 deletions rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -1939,19 +1939,20 @@ public byte[] getStateForBtcReleaseClient() throws IOException {

/**
* Retrieves the current SVP spend transaction state for the SVP client.
*
* <p>
* This method checks if there is an SVP spend transaction waiting for signatures, and if so, it serializes
* the state into RLP format. If no transaction is waiting, it returns an encoded empty RLP list.
* </p>
*
* @return A byte array representing the RLP-encoded state of the SVP spend transaction. If no transaction
* is waiting, returns an RLP-encoded empty list.
* is waiting, returns a double RLP-encoded empty list.
*/
public byte[] getStateForSvpClient() {
return provider.getSvpSpendTxWaitingForSignatures()
.map(StateForProposedFederator::new)
.map(StateForProposedFederator::encodeToRlp)
.orElse(RLP.encodedEmptyList());
.orElse(RLP.encodeList(RLP.encodedEmptyList()));
}

/**
Expand Down
12 changes: 12 additions & 0 deletions rskj-core/src/main/java/co/rsk/peg/StateForFederator.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ public StateForFederator(byte[] rlpData, NetworkParameters networkParameters) {
decodeRlpToMap(rlpData), networkParameters));
}

/**
* Retrieves a sorted map of RSK transactions that are waiting for signatures.
*
* <p>
* The returned {@code SortedMap<Keccak256, BtcTransaction>} contains entries where the key
* is the hash of the RSK transaction, and the value is the {@code BtcTransaction} object.
* This method guarantees a non-null result, returning an empty map if no transactions are pending.
* </p>
*
* @return a non-null {@code SortedMap<Keccak256, BtcTransaction>} of transactions waiting for signatures;
* if no transactions are pending, an empty map is returned.
*/
public SortedMap<Keccak256, BtcTransaction> getRskTxsWaitingForSignatures() {
return rskTxsWaitingForSignatures;
}
Expand Down
18 changes: 13 additions & 5 deletions rskj-core/src/main/java/co/rsk/peg/StateForProposedFederator.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import co.rsk.bitcoinj.core.BtcTransaction;
import co.rsk.bitcoinj.core.NetworkParameters;
import co.rsk.crypto.Keccak256;
import java.util.AbstractMap;
import java.util.Map;
import java.util.Objects;
import org.ethereum.util.RLP;
Expand All @@ -32,10 +31,7 @@ public class StateForProposedFederator {
private final Map.Entry<Keccak256, BtcTransaction> svpSpendTxWaitingForSignatures;

public StateForProposedFederator(Map.Entry<Keccak256, BtcTransaction> svpSpendTxWaitingForSignatures) {
Objects.requireNonNull(svpSpendTxWaitingForSignatures);

this.svpSpendTxWaitingForSignatures =
new AbstractMap.SimpleImmutableEntry<>(svpSpendTxWaitingForSignatures);
this.svpSpendTxWaitingForSignatures = svpSpendTxWaitingForSignatures;
}

public StateForProposedFederator(byte[] rlpData, NetworkParameters networkParameters) {
Expand All @@ -44,6 +40,18 @@ public StateForProposedFederator(byte[] rlpData, NetworkParameters networkParame
decodeRlpToEntry(rlpData), networkParameters));
}

/**
* Retrieves the SVP spend transaction waiting for signatures.
*
* <p>
* This method returns a {@code Map.Entry<Keccak256, BtcTransaction>} representing
* the transaction waiting for signatures, with the hash of the transaction as the key
* and the {@code BtcTransaction} object as the value.
* </p>
*
* @return a {@code Map.Entry<Keccak256, BtcTransaction>} if a transaction is pending
* for signatures, or {@code null} if no such transaction exists.
*/
public Map.Entry<Keccak256, BtcTransaction> getSvpSpendTxWaitingForSignatures() {
return svpSpendTxWaitingForSignatures;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,9 @@
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.EmptySource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.NullSource;

import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.time.Instant;
Expand Down Expand Up @@ -213,9 +211,18 @@ void serializeAndDeserializeRskTxWaitingForSignatures_whenValidData_shouldReturn
assertArrayEquals(pegoutTx.bitcoinSerialize(), deserializedEntry.getValue().bitcoinSerialize());
}

@Test
void serializeRskTxWaitingForSignatures_whenNullValuePassed_shouldReturnEmptyResult() {
// Act
byte[] result =
BridgeSerializationUtils.serializeRskTxWaitingForSignatures(null);

// Assert
assertEquals(RLP.encodedEmptyList(), result);
}

@ParameterizedTest
@NullSource
@EmptySource
@MethodSource("provideInvalidData")
void deserializeRskTxWaitingForSignatures_whenInvalidData_shouldReturnEmptyResult(byte[] data) {
// Act
Map.Entry<Keccak256, BtcTransaction> result =
Expand All @@ -224,6 +231,13 @@ void deserializeRskTxWaitingForSignatures_whenInvalidData_shouldReturnEmptyResul
// Assert
assertNull(result);
}

private static Stream<byte[]> provideInvalidData() {
return Stream.of(
null,
new byte[]{},
RLP.encodedEmptyList());
}

@Test
void serializeAndDeserializeRskTxsWaitingForSignatures_whenValidEntries_shouldReturnEqualsResults() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,20 @@
* You should have received a copy of the GNU Lesser General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package co.rsk.peg;

import static co.rsk.RskTestUtils.createHash;
import static org.ethereum.TestUtils.assertThrows;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

import co.rsk.bitcoinj.core.BtcTransaction;
import co.rsk.bitcoinj.core.NetworkParameters;
import co.rsk.crypto.Keccak256;
import java.util.AbstractMap;
import java.util.Map;
import org.ethereum.util.RLP;
import org.junit.jupiter.api.Test;

class StateForProposedFederatorTest {
Expand All @@ -53,9 +54,15 @@ void stateForProposedFederator_whenTxWaitingForSignaturesAndSerializeAndDeserial
}

@Test
void stateForProposedFederator_whenNullValueAndSerializeAndDeserialize_shouldThrowNullPointerException() {
void stateForProposedFederator_whenEmptyDoubleRLPEncodedList_shouldReturnEmptySvpSpendTxWaitingForSignatures() {
// Arrange
var doubleEncodedEmptyList = RLP.encodeList(RLP.encodedEmptyList());

// Act
StateForProposedFederator deserializedStateForProposedFederator =
new StateForProposedFederator(doubleEncodedEmptyList, NETWORK_PARAMETERS);

// Assert
assertThrows(NullPointerException.class, () -> new StateForProposedFederator(null));
assertThrows(NullPointerException.class, () -> new StateForProposedFederator(null, NETWORK_PARAMETERS));
assertNull(deserializedStateForProposedFederator.getSvpSpendTxWaitingForSignatures());
}
}

0 comments on commit dcd8a9e

Please sign in to comment.