From f8609a3d01d6251abde967cf5719e9b7ce5d504b Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 31 Jan 2024 11:03:15 -0500 Subject: [PATCH] Add test for updated tx receipt processing logic which removes the txhash from dkg storage whenever the pending tx hash is determined to have been actually mined, either successfully (status=1) or not. If the tx is already mined then the Coordinator contract should be the source of truth for how to proceed. --- nucypher/datastore/dkg.py | 8 ++- tests/unit/test_ritualist.py | 130 ++++++++++++++++++++++++++++++++--- 2 files changed, 125 insertions(+), 13 deletions(-) diff --git a/nucypher/datastore/dkg.py b/nucypher/datastore/dkg.py index 8c1658b6e..f52359464 100644 --- a/nucypher/datastore/dkg.py +++ b/nucypher/datastore/dkg.py @@ -24,9 +24,11 @@ class DKGStorage: def store_transcript_txhash(self, ritual_id: int, txhash: HexBytes) -> None: self.data["transcript_tx_hashes"][ritual_id] = txhash - def clear_transcript_txhash(self, ritual_id: int, txhash: HexBytes): + def clear_transcript_txhash(self, ritual_id: int, txhash: HexBytes) -> bool: if self.get_transcript_txhash(ritual_id) == txhash: del self.data["transcript_tx_hashes"][ritual_id] + return True + return False def get_transcript_txhash(self, ritual_id: int) -> Optional[HexBytes]: return self.data["transcript_tx_hashes"].get(ritual_id) @@ -47,9 +49,11 @@ class DKGStorage: def store_aggregation_txhash(self, ritual_id: int, txhash: HexBytes) -> None: self.data["aggregation_tx_hashes"][ritual_id] = txhash - def clear_aggregated_txhash(self, ritual_id: int, txhash: HexBytes): + def clear_aggregated_txhash(self, ritual_id: int, txhash: HexBytes) -> bool: if self.get_aggregation_txhash(ritual_id) == txhash: del self.data["aggregation_tx_hashes"][ritual_id] + return True + return False def get_aggregation_txhash(self, ritual_id: int) -> Optional[HexBytes]: return self.data["aggregation_tx_hashes"].get(ritual_id) diff --git a/tests/unit/test_ritualist.py b/tests/unit/test_ritualist.py index e4859f088..5e5e6f5e7 100644 --- a/tests/unit/test_ritualist.py +++ b/tests/unit/test_ritualist.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + import pytest from hexbytes import HexBytes @@ -20,8 +22,8 @@ def agent(mock_contract_agency, ursulas) -> MockCoordinatorAgent: if ursula.checksum_address == provider: return ursula.public_keys(RitualisticPower) - coordinator_agent.post_transcript = lambda *args, **kwargs: HexBytes("deadbeef") - coordinator_agent.post_aggregation = lambda *args, **kwargs: HexBytes("deadbeef") + coordinator_agent.post_transcript = lambda *args, **kwargs: HexBytes("deadbeef1") + coordinator_agent.post_aggregation = lambda *args, **kwargs: HexBytes("deadbeef2") coordinator_agent.get_provider_public_key = mock_get_provider_public_key return coordinator_agent @@ -140,13 +142,13 @@ def test_perform_round_1( lambda *args, **kwargs: Coordinator.RitualStatus.DKG_AWAITING_TRANSCRIPTS ) - tx_hash = ursula.perform_round_1( + original_tx_hash = ursula.perform_round_1( ritual_id=0, authority=random_address, participants=cohort, timestamp=0 ) - assert tx_hash is not None + assert original_tx_hash is not None # ensure tx hash is stored - assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) == tx_hash + assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) == original_tx_hash # try again tx_hash = ursula.perform_round_1( @@ -154,8 +156,62 @@ def test_perform_round_1( ) assert tx_hash is None # no execution since pending tx already present + # pending tx gets mined and removed from storage - receipt status is 1 + mock_receipt = {"status": 1} + with patch.object( + agent.blockchain.client, "get_transaction_receipt", return_value=mock_receipt + ): + tx_hash = ursula.perform_round_1( + ritual_id=0, authority=random_address, participants=cohort, timestamp=0 + ) + # no execution since pending tx was present and determined to be mined + assert tx_hash is None + # tx hash removed since tx receipt was obtained - outcome moving + # forward is represented on contract + assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) is None + + # reset tx hash + ursula.dkg_storage.store_transcript_txhash(ritual_id=0, txhash=original_tx_hash) + + # pending tx gets mined and removed from storage - receipt + # status is 0 i.e. evm revert - so use contract state which indicates + # to submit transcript + mock_receipt = {"status": 0} + with patch.object( + agent.blockchain.client, "get_transaction_receipt", return_value=mock_receipt + ): + with patch.object( + agent, "post_transcript", lambda *args, **kwargs: HexBytes("A1B1") + ): + mock_tx_hash = ursula.perform_round_1( + ritual_id=0, authority=random_address, participants=cohort, timestamp=0 + ) + # execution occurs because evm revert causes execution to be retried + assert mock_tx_hash == HexBytes("A1B1") + # tx hash changed since original tx hash removed due to status being 0 + # and new tx hash added + # forward is represented on contract + assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) == mock_tx_hash + assert ( + ursula.dkg_storage.get_transcript_txhash(ritual_id=0) + != original_tx_hash + ) + + # reset tx hash + ursula.dkg_storage.store_transcript_txhash(ritual_id=0, txhash=original_tx_hash) + + # don't clear if tx hash mismatched + assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) is not None + assert not ursula.dkg_storage.clear_transcript_txhash( + ritual_id=0, txhash=HexBytes("abcd") + ) + assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) is not None + # clear tx hash - ursula.dkg_storage.store_transcript_txhash(ritual_id=0, txhash=None) + assert ursula.dkg_storage.clear_transcript_txhash( + ritual_id=0, txhash=original_tx_hash + ) + assert ursula.dkg_storage.get_transcript_txhash(ritual_id=0) is None # participant already posted transcript participant = agent.get_participant( @@ -167,7 +223,8 @@ def test_perform_round_1( tx_hash = ursula.perform_round_1( ritual_id=0, authority=random_address, participants=cohort, timestamp=0 ) - assert tx_hash is None # no execution performed + # no execution performed since already posted transcript + assert tx_hash is None # participant no longer already posted aggregated transcript participant.transcript = bytes() @@ -238,18 +295,69 @@ def test_perform_round_2( ) mocker.patch("nucypher.crypto.ferveo.dkg.verify_aggregate") - tx_hash = ursula.perform_round_2(ritual_id=0, timestamp=0) - assert tx_hash is not None + original_tx_hash = ursula.perform_round_2(ritual_id=0, timestamp=0) + assert original_tx_hash is not None # check tx hash - assert ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) == tx_hash + assert ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) == original_tx_hash # try again tx_hash = ursula.perform_round_2(ritual_id=0, timestamp=0) assert tx_hash is None # no execution since pending tx already present + # pending tx gets mined and removed from storage - receipt status is 1 + mock_receipt = {"status": 1} + with patch.object( + agent.blockchain.client, "get_transaction_receipt", return_value=mock_receipt + ): + tx_hash = ursula.perform_round_2(ritual_id=0, timestamp=0) + # no execution since pending tx was present and determined to be mined + assert tx_hash is None + # tx hash removed since tx receipt was obtained - outcome moving + # forward is represented on contract + assert ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) is None + + # reset tx hash + ursula.dkg_storage.store_aggregation_txhash(ritual_id=0, txhash=original_tx_hash) + + # pending tx gets mined and removed from storage - receipt + # status is 0 i.e. evm revert - so use contract state which indicates + # to submit transcript + mock_receipt = {"status": 0} + with patch.object( + agent.blockchain.client, "get_transaction_receipt", return_value=mock_receipt + ): + with patch.object( + agent, "post_aggregation", lambda *args, **kwargs: HexBytes("A1B1") + ): + mock_tx_hash = ursula.perform_round_2(ritual_id=0, timestamp=0) + # execution occurs because evm revert causes execution to be retried + assert mock_tx_hash == HexBytes("A1B1") + # tx hash changed since original tx hash removed due to status being 0 + # and new tx hash added + # forward is represented on contract + assert ( + ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) == mock_tx_hash + ) + assert ( + ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) + != original_tx_hash + ) + + # reset tx hash + ursula.dkg_storage.store_aggregation_txhash(ritual_id=0, txhash=original_tx_hash) + + # don't clear if tx hash mismatched + assert not ursula.dkg_storage.clear_aggregated_txhash( + ritual_id=0, txhash=HexBytes("1234") + ) + assert ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) is not None + # clear tx hash - ursula.dkg_storage.store_aggregation_txhash(ritual_id=0, txhash=None) + assert ursula.dkg_storage.clear_aggregated_txhash( + ritual_id=0, txhash=original_tx_hash + ) + assert ursula.dkg_storage.get_aggregation_txhash(ritual_id=0) is None # participant already posted aggregated transcript participant = agent.get_participant(