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

Jump to bottom

Register spend tx #2851

Merged
merged 4 commits into from
Nov 21, 2024
Merged

Register spend tx #2851

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3513c98
Modify public method to remove signatures for just returning the hash…
julia-zack Nov 19, 2024
30dafce
Register spend transaction
julia-zack Nov 19, 2024
007ffad
Create new registerSvpSpendTransaction method. Refactor to make code …
julia-zack Nov 20, 2024
29bc369
Add test for getTxHashWithoutSigs using a real pegout. Minor refactor
julia-zack Nov 21, 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
44 changes: 26 additions & 18 deletions rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,14 @@ public void registerBtcTransaction(
throw new RegisterBtcTransactionException("Transaction already processed");
}

if (svpIsOngoing() && isTheSvpSpendTransaction(btcTx)) {
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
markTxAsProcessed(btcTx);
saveNewUTXOs(btcTx);
provider.setSvpSpendTxHashUnsigned(null);
// proceed with svp success
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will be addressed in next task

nathanieliov marked this conversation as resolved.
Show resolved Hide resolved
return;
}

PegTxType pegTxType = PegUtils.getTransactionType(
activations,
provider,
Expand All @@ -410,8 +418,8 @@ public void registerBtcTransaction(
case PEGOUT_OR_MIGRATION:
logger.debug("[registerBtcTransaction] This is a peg-out or migration tx {}", btcTx.getHash());
processPegoutOrMigration(btcTx);
if (svpIsOngoing()) {
updateSvpFundTransactionValuesIfPossible(btcTx);
if (svpIsOngoing() && isTheSvpFundTransaction(btcTx)) {
nathanieliov marked this conversation as resolved.
Show resolved Hide resolved
updateSvpFundTransactionValues(btcTx);
}
break;
default:
Expand All @@ -429,21 +437,13 @@ public void registerBtcTransaction(
}
}

private void updateSvpFundTransactionValuesIfPossible(BtcTransaction transaction) {
provider.getSvpFundTxHashUnsigned()
.filter(svpFundTxHashUnsigned -> isTheSvpFundTransaction(svpFundTxHashUnsigned, transaction))
.ifPresent(isTheSvpFundTransaction -> updateSvpFundTransactionValues(transaction));
}

private boolean isTheSvpFundTransaction(Sha256Hash svpFundTransactionHashUnsigned, BtcTransaction transaction) {
Sha256Hash transactionHash = transaction.getHash();

if (!transaction.hasWitness()) {
BtcTransaction transactionCopyWithoutSignatures = new BtcTransaction(networkParameters, transaction.bitcoinSerialize()); // this is needed to not remove signatures from the actual tx
BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transactionCopyWithoutSignatures);
transactionHash = transactionCopyWithoutSignatures.getHash();
}
return transactionHash.equals(svpFundTransactionHashUnsigned);
private boolean isTheSvpFundTransaction(BtcTransaction transaction) {
return provider.getSvpFundTxHashUnsigned()
.map(svpFundTransactionHashUnsigned -> {
Sha256Hash transactionHashWithoutSignatures = getMultiSigTransactionHashWithoutSignatures(networkParameters, transaction);
return transactionHashWithoutSignatures.equals(svpFundTransactionHashUnsigned);
})
.orElse(false);
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
}

private void updateSvpFundTransactionValues(BtcTransaction transaction) {
Expand All @@ -455,6 +455,15 @@ private void updateSvpFundTransactionValues(BtcTransaction transaction) {
provider.setSvpFundTxHashUnsigned(null);
}

private boolean isTheSvpSpendTransaction(BtcTransaction transaction) {
return provider.getSvpSpendTxHashUnsigned()
.map(svpSpendTransactionHashUnsigned -> {
Sha256Hash transactionHashWithoutSignatures = getMultiSigTransactionHashWithoutSignatures(networkParameters, transaction);
return transactionHashWithoutSignatures.equals(svpSpendTransactionHashUnsigned);
})
.orElse(false);
}

private Script getLastRetiredFederationP2SHScript() {
return federationSupport.getLastRetiredFederationP2SHScript().orElse(null);
}
Expand Down Expand Up @@ -1760,7 +1769,6 @@ private void addSvpSpendTxSignatures(
provider.getSvpSpendTxWaitingForSignatures()
// The svpSpendTxWFS should always be present at this point, since we already checked isTheSvpSpendTx.
.ifPresent(svpSpendTxWFS -> {

Keccak256 svpSpendTxCreationRskTxHash = svpSpendTxWFS.getKey();
BtcTransaction svpSpendTx = svpSpendTxWFS.getValue();

Expand Down
14 changes: 13 additions & 1 deletion rskj-core/src/main/java/co/rsk/peg/bitcoin/BitcoinUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,19 @@ public static Optional<Script> extractRedeemScriptFromInput(TransactionInput txI
}
}

public static void removeSignaturesFromTransactionWithP2shMultiSigInputs(BtcTransaction transaction) {
public static Sha256Hash getMultiSigTransactionHashWithoutSignatures(NetworkParameters networkParameters, BtcTransaction transaction) {
Sha256Hash transactionHash = transaction.getHash();

if (!transaction.hasWitness()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!transaction.hasWitness()) {
if (transaction.hasWitness()) {
return transaction.getHash();
}
...

Nit, just to return early

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i put it this way considering the txs are normally segwit (correct me if im wrong! :) ), so we will have the common behavior outside the check

BtcTransaction transactionCopyWithoutSignatures = new BtcTransaction(networkParameters, transaction.bitcoinSerialize()); // this is needed to not remove signatures from the actual tx
BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transactionCopyWithoutSignatures);
transactionHash = transactionCopyWithoutSignatures.getHash();
}

return transactionHash;
julia-zack marked this conversation as resolved.
Show resolved Hide resolved
}

private static void removeSignaturesFromTransactionWithP2shMultiSigInputs(BtcTransaction transaction) {
if (transaction.hasWitness()) {
String message = "Removing signatures from SegWit transactions is not allowed.";
logger.error("[removeSignaturesFromTransactionWithP2shMultiSigInputs] {}", message);
Expand Down
187 changes: 146 additions & 41 deletions rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ public class BridgeSupportSvpTest {
private Sha256Hash svpSpendTransactionHashUnsigned;
private BtcTransaction svpSpendTransaction;

private PartialMerkleTree pmtWithTransactions;
private int btcBlockWithPmtHeight;

@BeforeEach
void setUp() {
long rskExecutionBlockNumber = 1000L;
Expand Down Expand Up @@ -467,8 +470,6 @@ private void assertSvpFundTransactionValuesWereNotUpdated() {
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@Tag("Fund transaction registration tests")
class FundTxRegistrationTests {
private PartialMerkleTree pmtWithTransactions;
private int btcBlockWithPmtHeight;

@Test
void registerBtcTransaction_forSvpFundTransactionChange_whenProposedFederationDoesNotExist_shouldRegisterTransactionButNotUpdateSvpFundTransactionValues() throws Exception {
Expand Down Expand Up @@ -595,43 +596,6 @@ void registerBtcTransaction_forSvpFundTransactionChange_whenSvpPeriodIsOngoing_s
assertSvpFundTransactionValuesWereUpdated();
}

private void setUpForTransactionRegistration(BtcTransaction transaction) throws BlockStoreException {
// recreate a valid chain that has the tx, so it passes the previous checks in registerBtcTransaction
BtcBlockStoreWithCache.Factory btcBlockStoreFactory = new RepositoryBtcBlockStoreWithCache.Factory(btcMainnetParams, 100, 100);
BtcBlockStoreWithCache btcBlockStoreWithCache = btcBlockStoreFactory.newInstance(repository, bridgeMainNetConstants, bridgeStorageProvider, allActivations);

pmtWithTransactions = createValidPmtForTransactions(Collections.singletonList(transaction.getHash()), btcMainnetParams);
btcBlockWithPmtHeight = bridgeMainNetConstants.getBtcHeightWhenPegoutTxIndexActivates() + bridgeMainNetConstants.getPegoutTxIndexGracePeriodInBtcBlocks(); // we want pegout tx index to be activated

int chainHeight = btcBlockWithPmtHeight + bridgeMainNetConstants.getBtc2RskMinimumAcceptableConfirmations();
recreateChainFromPmt(btcBlockStoreWithCache, chainHeight, pmtWithTransactions, btcBlockWithPmtHeight, btcMainnetParams);

bridgeStorageProvider.save();

bridgeSupport = bridgeSupportBuilder
.withBridgeConstants(bridgeMainNetConstants)
.withProvider(bridgeStorageProvider)
.withActivations(allActivations)
.withFederationSupport(federationSupport)
.withFeePerKbSupport(feePerKbSupport)
.withExecutionBlock(rskExecutionBlock)
.withBtcBlockStoreFactory(btcBlockStoreFactory)
.withRepository(repository)
.build();
}

private void assertActiveFederationUtxosSize(int expectedActiveFederationUtxosSize) {
assertEquals(expectedActiveFederationUtxosSize, federationSupport.getActiveFederationBtcUTXOs().size());
}

private void assertTransactionWasProcessed(Sha256Hash transactionHash) throws IOException {
Optional<Long> rskBlockHeightAtWhichBtcTxWasProcessed = bridgeStorageProvider.getHeightIfBtcTxhashIsAlreadyProcessed(transactionHash);
assertTrue(rskBlockHeightAtWhichBtcTxWasProcessed.isPresent());

long rskExecutionBlockNumber = rskExecutionBlock.getNumber();
assertEquals(rskExecutionBlockNumber, rskBlockHeightAtWhichBtcTxWasProcessed.get());
}

Comment on lines -598 to -634
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was just moved outside to be reused

private void assertSvpFundTransactionValuesWereUpdated() {
Optional<BtcTransaction> svpFundTransactionSignedOpt = bridgeStorageProvider.getSvpFundTxSigned();
assertTrue(svpFundTransactionSignedOpt.isPresent());
Expand Down Expand Up @@ -666,7 +630,7 @@ void updateCollections_whenSpendTxCanBeCreated_createsExpectedSpendTxAndSavesThe
bridgeStorageProvider.save();

// assert
svpSpendTransactionHashUnsigned = assertSvpSpendTxHashUnsignedWasSavedInStorage();
svpSpendTransactionHashUnsigned = assertSvpSpendTxHashUnsignedIsSavedInStorage();
svpSpendTransaction = assertSvpSpendTxIsWaitingForSignatures();
assertSvpSpendTxHasExpectedInputsAndOutputs();

Expand Down Expand Up @@ -727,7 +691,7 @@ private void assertInputHasExpectedScriptSig(TransactionInput input, Script rede
}
}

private Sha256Hash assertSvpSpendTxHashUnsignedWasSavedInStorage() {
private Sha256Hash assertSvpSpendTxHashUnsignedIsSavedInStorage() {
Optional<Sha256Hash> svpSpendTransactionHashUnsignedOpt = bridgeStorageProvider.getSvpSpendTxHashUnsigned();
assertTrue(svpSpendTransactionHashUnsignedOpt.isPresent());

Expand Down Expand Up @@ -982,6 +946,105 @@ private void assertFederatorSignedInputs(List<TransactionInput> inputs, List<Sha
}
}

@Nested
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@Tag("Spend transaction registration tests")
class SpendTxRegistrationTests {
@Test
void registerBtcTransaction_whenValidationPeriodEnded_shouldNotProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
// arrange
arrangeExecutionBlockIsAfterValidationPeriodEnded();

arrangeSvpSpendTransaction();
setUpForTransactionRegistration(svpSpendTransaction);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();
bridgeSupport.registerBtcTransaction(
rskTx,
svpSpendTransaction.bitcoinSerialize(),
btcBlockWithPmtHeight,
pmtWithTransactions.bitcoinSerialize()
);
bridgeStorageProvider.save();

// assert
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx);
assertTransactionWasNotProcessed(svpSpendTransaction.getHash());
assertSvpSpendTxHashUnsignedIsSavedInStorage();
}

@Test
void registerBtcTransaction_whenIsTheSpendTransaction_shouldProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
// arrange
arrangeSvpSpendTransaction();
setUpForTransactionRegistration(svpSpendTransaction);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();
bridgeSupport.registerBtcTransaction(
rskTx,
svpSpendTransaction.bitcoinSerialize(),
btcBlockWithPmtHeight,
pmtWithTransactions.bitcoinSerialize()
);
bridgeStorageProvider.save();

// assert
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx + 1);
assertTransactionWasProcessed(svpSpendTransaction.getHash());
assertNoSvpSpendTxHash();
}

@Test
void registerBtcTransaction_whenIsNotTheSpendTransaction_shouldNotProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
// arrange
arrangeSvpSpendTransaction();
setUpForTransactionRegistration(svpSpendTransaction);

BtcTransaction pegout = createPegout(proposedFederation.getRedeemScript());
savePegoutIndex(pegout);
signInputs(pegout);
setUpForTransactionRegistration(pegout);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();
bridgeSupport.registerBtcTransaction(
rskTx,
pegout.bitcoinSerialize(),
btcBlockWithPmtHeight,
pmtWithTransactions.bitcoinSerialize()
);
bridgeStorageProvider.save();

// assert
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx + 1);
assertTransactionWasNotProcessed(svpSpendTransaction.getHash());
assertSvpSpendTxHashUnsignedIsSavedInStorage();
}

@Test
void registerBtcTransaction_whenSpendTransactionHashIsNotSaved_shouldNotProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
// arrange
recreateSvpSpendTransactionUnsigned();
setUpForTransactionRegistration(svpSpendTransaction);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();
bridgeSupport.registerBtcTransaction(
rskTx,
svpSpendTransaction.bitcoinSerialize(),
btcBlockWithPmtHeight,
pmtWithTransactions.bitcoinSerialize()
);
bridgeStorageProvider.save();

// assert
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx);
assertTransactionWasNotProcessed(svpSpendTransaction.getHash());
}
}

private void arrangeExecutionBlockIsAfterValidationPeriodEnded() {
long validationPeriodEndBlock = proposedFederation.getCreationBlockNumber()
+ bridgeMainNetConstants.getFederationConstants().getValidationPeriodDurationInBlocks();
Expand Down Expand Up @@ -1081,6 +1144,48 @@ private void saveSvpSpendTransactionValues() {
bridgeStorageProvider.save();
}

private void setUpForTransactionRegistration(BtcTransaction transaction) throws BlockStoreException {
// recreate a valid chain that has the tx, so it passes the previous checks in registerBtcTransaction
BtcBlockStoreWithCache.Factory btcBlockStoreFactory = new RepositoryBtcBlockStoreWithCache.Factory(btcMainnetParams, 100, 100);
BtcBlockStoreWithCache btcBlockStoreWithCache = btcBlockStoreFactory.newInstance(repository, bridgeMainNetConstants, bridgeStorageProvider, allActivations);

pmtWithTransactions = createValidPmtForTransactions(Collections.singletonList(transaction.getHash()), btcMainnetParams);
btcBlockWithPmtHeight = bridgeMainNetConstants.getBtcHeightWhenPegoutTxIndexActivates() + bridgeMainNetConstants.getPegoutTxIndexGracePeriodInBtcBlocks(); // we want pegout tx index to be activated

int chainHeight = btcBlockWithPmtHeight + bridgeMainNetConstants.getBtc2RskMinimumAcceptableConfirmations();
recreateChainFromPmt(btcBlockStoreWithCache, chainHeight, pmtWithTransactions, btcBlockWithPmtHeight, btcMainnetParams);

bridgeStorageProvider.save();

bridgeSupport = bridgeSupportBuilder
.withBridgeConstants(bridgeMainNetConstants)
.withProvider(bridgeStorageProvider)
.withActivations(allActivations)
.withFederationSupport(federationSupport)
.withFeePerKbSupport(feePerKbSupport)
.withExecutionBlock(rskExecutionBlock)
.withBtcBlockStoreFactory(btcBlockStoreFactory)
.withRepository(repository)
.build();
}

private void assertActiveFederationUtxosSize(int expectedActiveFederationUtxosSize) {
assertEquals(expectedActiveFederationUtxosSize, federationSupport.getActiveFederationBtcUTXOs().size());
}

private void assertTransactionWasProcessed(Sha256Hash transactionHash) throws IOException {
Optional<Long> rskBlockHeightAtWhichBtcTxWasProcessed = bridgeStorageProvider.getHeightIfBtcTxhashIsAlreadyProcessed(transactionHash);
assertTrue(rskBlockHeightAtWhichBtcTxWasProcessed.isPresent());

long rskExecutionBlockNumber = rskExecutionBlock.getNumber();
assertEquals(rskExecutionBlockNumber, rskBlockHeightAtWhichBtcTxWasProcessed.get());
}

private void assertTransactionWasNotProcessed(Sha256Hash transactionHash) throws IOException {
Optional<Long> rskBlockHeightAtWhichBtcTxWasProcessed = bridgeStorageProvider.getHeightIfBtcTxhashIsAlreadyProcessed(transactionHash);
assertFalse(rskBlockHeightAtWhichBtcTxWasProcessed.isPresent());
}

private void assertNoSvpFundTxHashUnsigned() {
assertFalse(bridgeStorageProvider.getSvpFundTxHashUnsigned().isPresent());
}
Expand Down
Loading