From 0aed191763640c7b8d2a70a04adaef99b98d9c27 Mon Sep 17 00:00:00 2001 From: julia-zack Date: Wed, 22 Jan 2025 09:46:14 -0300 Subject: [PATCH 1/3] Derive federation creation time correctly (from seconds instead of milliseconds) --- .../main/java/co/rsk/peg/federation/FederationSupportImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java b/rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java index 75d1c9d15d..7c08a132d0 100644 --- a/rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java @@ -767,7 +767,7 @@ private FederationChangeResponseCode commitPendingFederation(PendingFederation c } private Federation buildFederationFromPendingFederation(PendingFederation pendingFederation) { - Instant federationCreationTime = Instant.ofEpochMilli(rskExecutionBlock.getTimestamp()); + Instant federationCreationTime = Instant.ofEpochSecond(rskExecutionBlock.getTimestamp()); long federationCreationBlockNumber = rskExecutionBlock.getNumber(); return pendingFederation.buildFederation(federationCreationTime, federationCreationBlockNumber, constants, activations); From 839fc5d0d6ace379ec71e2de5dcfb410fe15fe0a Mon Sep 17 00:00:00 2001 From: julia-zack Date: Wed, 22 Jan 2025 19:32:03 -0300 Subject: [PATCH 2/3] Add tests --- .../federation/VoteFederationChangeTest.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java b/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java index e5195f116c..504ff4c843 100644 --- a/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java @@ -416,15 +416,15 @@ void voteCommitFederation_postRSKIP186_preRSKIP419_whenPendingFederationIsSet_sh voteAndAssertCommitPendingFederation(); // assertions + Federation newFederation = storageProvider.getNewFederation(federationMainnetConstants, activations); + assertIsTheExpectedFederation(newFederation); + List utxosToMove = List.copyOf(storageProvider.getNewFederationBtcUTXOs(federationMainnetConstants.getBtcParams(), activations)); - Federation federationBuiltFromPendingFederation = - pendingFederationToBe.buildFederation(Instant.ofEpochMilli(RSK_EXECUTION_BLOCK_TIMESTAMP), RSK_EXECUTION_BLOCK_NUMBER, federationMainnetConstants, activations); - assertHandoverToNewFederation(utxosToMove, federationBuiltFromPendingFederation); + assertHandoverToNewFederation(utxosToMove, newFederation); assertPendingFederationVotingWasCleaned(); Federation oldFederation = storageProvider.getOldFederation(federationMainnetConstants, activations); - Federation newFederation = storageProvider.getNewFederation(federationMainnetConstants, activations); assertLogCommitFederation(oldFederation, newFederation); } @@ -435,9 +435,6 @@ void voteCommitFederation_postRSKIP419_whenPendingFederationIsSet_shouldPerformC List activeFederationUTXOs = BitcoinTestUtils.createUTXOs(10, activeFederation.getAddress()); bridgeStorageAccessor.saveToRepository(NEW_FEDERATION_BTC_UTXOS_KEY.getKey(), activeFederationUTXOs, BridgeSerializationUtils::serializeUTXOList); - Federation federationBuiltFromPendingFederation = - pendingFederationToBe.buildFederation(Instant.ofEpochMilli(RSK_EXECUTION_BLOCK_TIMESTAMP), RSK_EXECUTION_BLOCK_NUMBER, federationMainnetConstants, activations); - voteAndAssertCreateEmptyPendingFederation(); voteAndAssertAddFederationMembersPublicKeysToPendingFederation(pendingFederationToBe.getMembers()); @@ -448,7 +445,7 @@ void voteCommitFederation_postRSKIP419_whenPendingFederationIsSet_shouldPerformC // assert proposed federation was set correctly Optional proposedFederation = storageProvider.getProposedFederation(federationMainnetConstants, activations); assertTrue(proposedFederation.isPresent()); - assertEquals(federationBuiltFromPendingFederation, proposedFederation.get()); + assertIsTheExpectedFederation(proposedFederation.get()); assertPendingFederationVotingWasCleaned(); @@ -457,6 +454,20 @@ void voteCommitFederation_postRSKIP419_whenPendingFederationIsSet_shouldPerformC assertNoHandoverToNewFederation(); } + private void assertIsTheExpectedFederation(Federation federation) { + Federation federationBuiltFromPendingFederation = pendingFederationToBe.buildFederation( + Instant.ofEpochSecond(RSK_EXECUTION_BLOCK_TIMESTAMP), + RSK_EXECUTION_BLOCK_NUMBER, + federationMainnetConstants, + activations + ); + + assertEquals(federationBuiltFromPendingFederation.getCreationTime(), federation.getCreationTime()); + assertEquals(federationBuiltFromPendingFederation.getCreationBlockNumber(), federation.getCreationBlockNumber()); + + assertEquals(federationBuiltFromPendingFederation, federation); + } + @Test void commitProposedFederation_shouldPerformCommitProposedFederationActions() { // arrange From 57a9edd475b4204b890dec22e6f983fcf7d74438 Mon Sep 17 00:00:00 2001 From: julia-zack Date: Thu, 23 Jan 2025 11:21:34 -0300 Subject: [PATCH 3/3] Make test assertion more explicit --- .../co/rsk/peg/federation/VoteFederationChangeTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java b/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java index 504ff4c843..c067ae2068 100644 --- a/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java @@ -455,16 +455,15 @@ void voteCommitFederation_postRSKIP419_whenPendingFederationIsSet_shouldPerformC } private void assertIsTheExpectedFederation(Federation federation) { + assertEquals(RSK_EXECUTION_BLOCK_TIMESTAMP, federation.getCreationTime().getEpochSecond()); + assertEquals(RSK_EXECUTION_BLOCK_NUMBER, federation.getCreationBlockNumber()); + Federation federationBuiltFromPendingFederation = pendingFederationToBe.buildFederation( Instant.ofEpochSecond(RSK_EXECUTION_BLOCK_TIMESTAMP), RSK_EXECUTION_BLOCK_NUMBER, federationMainnetConstants, activations ); - - assertEquals(federationBuiltFromPendingFederation.getCreationTime(), federation.getCreationTime()); - assertEquals(federationBuiltFromPendingFederation.getCreationBlockNumber(), federation.getCreationBlockNumber()); - assertEquals(federationBuiltFromPendingFederation, federation); }