Skip to content

Commit

Permalink
Refactor tests and improve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@julia-zack
julia-zack committed Nov 21, 2024
1 parent 41fccdc commit d55617b
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 67 deletions.
16 changes: 9 additions & 7 deletions rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ private FederationChangeResponseCode commitPendingFederationAccordingToActivatio
*/
private FederationChangeResponseCode legacyCommitPendingFederation(PendingFederation currentPendingFederation, BridgeEventLogger eventLogger) {
Federation newFederation = buildFederationFromPendingFederation(currentPendingFederation);
handoverFromOldToNewFederation(getActiveFederation(), newFederation);
handoverToNewFederation(newFederation);

clearPendingFederationVoting();

Expand All @@ -708,14 +708,14 @@ public void commitProposedFederation() {
Federation proposedFederation = provider.getProposedFederation(constants, activations)
.orElseThrow(IllegalStateException::new);

handoverFromOldToNewFederation(getActiveFederation(), proposedFederation);
handoverToNewFederation(proposedFederation);
clearProposedFederation();
}

private void handoverFromOldToNewFederation(Federation oldFederation, Federation newFederation) {
private void handoverToNewFederation(Federation newFederation) {
moveUTXOsFromNewToOldFederation();

setOldAndNewFederations(oldFederation, newFederation);
setOldAndNewFederations(getActiveFederation(), newFederation);

if (activations.isActive(RSKIP186)) {
saveLastRetiredFederationScript();
Expand All @@ -729,15 +729,17 @@ private void setOldAndNewFederations(Federation oldFederation, Federation newFed
}

private void moveUTXOsFromNewToOldFederation() {
List<UTXO> utxosToMove = new ArrayList<>(provider.getNewFederationBtcUTXOs(constants.getBtcParams(), activations));
// since the current active fed reference will change from being 'new' to 'old',
// we have to change the UTXOs reference to match it
List<UTXO> activeFederationUTXOs = new ArrayList<>(provider.getNewFederationBtcUTXOs(constants.getBtcParams(), activations));

// Clear new and old federation's UTXOs
provider.getNewFederationBtcUTXOs(constants.getBtcParams(), activations).clear();
List<UTXO> oldFederationUTXOs = provider.getOldFederationBtcUTXOs();
oldFederationUTXOs.clear();

// Move UTXOs from the new federation into the old federation
oldFederationUTXOs.addAll(utxosToMove);
// Move UTXOs reference to the old federation
oldFederationUTXOs.addAll(activeFederationUTXOs);
}

/**
Expand Down
123 changes: 94 additions & 29 deletions rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,8 +963,9 @@ void registerBtcTransaction_whenValidationPeriodEnded_shouldNotProcessSpendTx()
arrangeSvpSpendTransaction();
setUpForTransactionRegistration(svpSpendTransaction);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();

// act
bridgeSupport.registerBtcTransaction(
rskTx,
svpSpendTransaction.bitcoinSerialize(),
Expand All @@ -974,9 +975,14 @@ void registerBtcTransaction_whenValidationPeriodEnded_shouldNotProcessSpendTx()
bridgeStorageProvider.save();

// assert
// tx was not registered
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx);
assertTransactionWasNotProcessed(svpSpendTransaction.getHash());

// svp success was not processed
assertSvpSpendTxHashUnsignedIsSavedInStorage();
assertNoHandoverToNewFederation();
assertProposedFederation();
}

@Test
Expand All @@ -990,8 +996,9 @@ void registerBtcTransaction_whenIsNotTheSpendTransaction_shouldNotProcessSpendTx
signInputs(pegout);
setUpForTransactionRegistration(pegout);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();

// act
bridgeSupport.registerBtcTransaction(
rskTx,
pegout.bitcoinSerialize(),
Expand All @@ -1001,9 +1008,16 @@ void registerBtcTransaction_whenIsNotTheSpendTransaction_shouldNotProcessSpendTx
bridgeStorageProvider.save();

// assert
// pegout was registered
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx + 1);
assertTransactionWasProcessed(pegout.getHash());
// but spend tx was not
assertTransactionWasNotProcessed(svpSpendTransaction.getHash());

// svp success was not processed
assertSvpSpendTxHashUnsignedIsSavedInStorage();
assertNoHandoverToNewFederation();
assertProposedFederation();
}

@Test
Expand All @@ -1012,8 +1026,9 @@ void registerBtcTransaction_whenSpendTransactionHashIsNotSaved_shouldNotProcessS
recreateSvpSpendTransactionUnsigned();
setUpForTransactionRegistration(svpSpendTransaction);

// act
int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();

// act
bridgeSupport.registerBtcTransaction(
rskTx,
svpSpendTransaction.bitcoinSerialize(),
Expand All @@ -1023,8 +1038,17 @@ void registerBtcTransaction_whenSpendTransactionHashIsNotSaved_shouldNotProcessS
bridgeStorageProvider.save();

// assert
// spend tx was not registered
assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx);
assertTransactionWasNotProcessed(svpSpendTransaction.getHash());

// svp success was not processed
assertNoHandoverToNewFederation();
assertProposedFederation();
}

private void assertProposedFederation() {
assertTrue(federationSupport.getProposedFederation().isPresent());
}

@Test
Expand All @@ -1033,8 +1057,9 @@ void registerBtcTransaction_whenIsTheSpendTransaction_shouldProcessSpendTxAndSvp
arrangeSvpSpendTransaction();
setUpForTransactionRegistration(svpSpendTransaction);

List<UTXO> activeFederationUtxosBeforeRegisteringTx = new ArrayList<>(federationSupport.getActiveFederationBtcUTXOs());

// act
List<UTXO> activeFedUtxosBeforeRegisteringTx = new ArrayList<>(federationSupport.getActiveFederationBtcUTXOs());
bridgeSupport.registerBtcTransaction(
rskTx,
svpSpendTransaction.bitcoinSerialize(),
Expand All @@ -1045,18 +1070,13 @@ void registerBtcTransaction_whenIsTheSpendTransaction_shouldProcessSpendTxAndSvp

// assert
// tx registration
assertActiveFederationUtxosSize(activeFedUtxosBeforeRegisteringTx.size() + 1);
assertActiveFederationUtxosSize(activeFederationUtxosBeforeRegisteringTx.size() + 1);
assertTransactionWasProcessed(svpSpendTransaction.getHash());
assertNoSvpSpendTxHash();

// svp success
assertNoSvpSpendTxHash();
assertNoProposedFederation();

assertNewActiveFederationCreationBlockHeightWasSet();
assertLastRetiredFederationScriptWasSet();

assertNewAndOldFederationsWereSet();

int outputSentToActiveFedIndex = 0;
TransactionOutput spendTxOutputSentToActiveFed = svpSpendTransaction.getOutput(outputSentToActiveFedIndex);
UTXO utxoSentToActiveFederation = new UTXO(
Expand All @@ -1067,21 +1087,43 @@ void registerBtcTransaction_whenIsTheSpendTransaction_shouldProcessSpendTxAndSvp
svpSpendTransaction.isCoinBase(),
spendTxOutputSentToActiveFed.getScriptPubKey()
);
activeFedUtxosBeforeRegisteringTx.add(utxoSentToActiveFederation);
assertUTXOsWereMovedFromNewToOldFederation(activeFedUtxosBeforeRegisteringTx);
activeFederationUtxosBeforeRegisteringTx.add(utxoSentToActiveFederation);

assertHandoverToNewFederation(activeFederationUtxosBeforeRegisteringTx);
}

private void assertNewActiveFederationCreationBlockHeightWasSet() {
Optional<Long> nextFederationCreationBlockHeight = federationStorageProvider.getNextFederationCreationBlockHeight(allActivations);
assertTrue(nextFederationCreationBlockHeight.isPresent());
assertEquals(proposedFederation.getCreationBlockNumber(), nextFederationCreationBlockHeight.get());
private void assertHandoverToNewFederation(List<UTXO> utxosToMove) {
assertUTXOsWereMovedFromNewToOldFederation(utxosToMove);
assertNewAndOldFederationsWereSet();
assertLastRetiredFederationScriptWasSet();
assertNewActiveFederationCreationBlockHeightWasSet();
}

private void assertLastRetiredFederationScriptWasSet() {
Script activeFederationMembersP2SHScript = getFederationMembersP2SHScript(allActivations, federationSupport.getActiveFederation());
Optional<Script> lastRetiredFederationP2SHScript = federationStorageProvider.getLastRetiredFederationP2SHScript(allActivations);
assertTrue(lastRetiredFederationP2SHScript.isPresent());
assertEquals(activeFederationMembersP2SHScript, lastRetiredFederationP2SHScript.get());
private void assertNoHandoverToNewFederation() {
assertUTXOsWereNotMovedToOldFederation();
assertNewAndOldFederationsWereNotSet();
assertLastRetiredFederationScriptWasNotSet();
assertNewActiveFederationCreationBlockHeightWasNotSet();
}

private void assertUTXOsWereMovedFromNewToOldFederation(List<UTXO> utxosToMove) {
// assert utxos were moved from new federation to old federation
List<UTXO> oldFederationUTXOs = federationStorageProvider.getOldFederationBtcUTXOs();
assertEquals(utxosToMove, oldFederationUTXOs);

// assert new federation utxos were cleaned
List<UTXO> newFederationUTXOs = federationStorageProvider.getNewFederationBtcUTXOs(btcMainnetParams, allActivations);
assertTrue(newFederationUTXOs.isEmpty());
}

private void assertUTXOsWereNotMovedToOldFederation() {
// assert old federation utxos are still empty
List<UTXO> oldFederationUTXOs = federationStorageProvider.getOldFederationBtcUTXOs();
assertTrue(oldFederationUTXOs.isEmpty());

// assert new federation utxos are not empty
List<UTXO> newFederationUTXOs = federationStorageProvider.getNewFederationBtcUTXOs(btcMainnetParams, allActivations);
assertFalse(newFederationUTXOs.isEmpty());
}

private void assertNewAndOldFederationsWereSet() {
Expand All @@ -1094,14 +1136,37 @@ private void assertNewAndOldFederationsWereSet() {
assertEquals(proposedFederation, newFederation);
}

private void assertUTXOsWereMovedFromNewToOldFederation(List<UTXO> utxosToMove) {
// assert utxos were moved from new federation to old federation
List<UTXO> oldFederationUTXOs = federationStorageProvider.getOldFederationBtcUTXOs();
assertEquals(utxosToMove, oldFederationUTXOs);
private void assertNewAndOldFederationsWereNotSet() {
// assert old federation is still null
Federation oldFederation = federationStorageProvider.getOldFederation(federationMainNetConstants, allActivations);
assertNull(oldFederation);

// assert new federation utxos were cleaned
List<UTXO> newFederationUTXOs = federationStorageProvider.getNewFederationBtcUTXOs(btcMainnetParams, allActivations);
assertTrue(newFederationUTXOs.isEmpty());
// assert new federation is still the active federation
Federation newFederation = federationStorageProvider.getNewFederation(federationMainNetConstants, allActivations);
assertEquals(activeFederation, newFederation);
}

private void assertLastRetiredFederationScriptWasSet() {
Script activeFederationMembersP2SHScript = getFederationMembersP2SHScript(allActivations, federationSupport.getActiveFederation());
Optional<Script> lastRetiredFederationP2SHScript = federationStorageProvider.getLastRetiredFederationP2SHScript(allActivations);
assertTrue(lastRetiredFederationP2SHScript.isPresent());
assertEquals(activeFederationMembersP2SHScript, lastRetiredFederationP2SHScript.get());
}

private void assertLastRetiredFederationScriptWasNotSet() {
Optional<Script> lastRetiredFederationP2SHScript = federationStorageProvider.getLastRetiredFederationP2SHScript(allActivations);
assertFalse(lastRetiredFederationP2SHScript.isPresent());
}

private void assertNewActiveFederationCreationBlockHeightWasSet() {
Optional<Long> nextFederationCreationBlockHeight = federationStorageProvider.getNextFederationCreationBlockHeight(allActivations);
assertTrue(nextFederationCreationBlockHeight.isPresent());
assertEquals(proposedFederation.getCreationBlockNumber(), nextFederationCreationBlockHeight.get());
}

private void assertNewActiveFederationCreationBlockHeightWasNotSet() {
Optional<Long> nextFederationCreationBlockHeight = federationStorageProvider.getNextFederationCreationBlockHeight(allActivations);
assertFalse(nextFederationCreationBlockHeight.isPresent());
}
}

Expand Down
Loading

0 comments on commit d55617b

Please sign in to comment.