[INDY-1983] Tracking PP first validation time to check their obsolescences #1155

andkononykhin · 2019-04-08T17:38:48Z

No description provided.

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

plenum/server/replica.py

ashcherbakov · 2019-04-09T09:09:47Z

plenum/server/replica.py

@@ -1049,11 +1057,17 @@ def process_three_phase_msg(self, msg: ThreePhaseMsg, sender: str):
        """
        sender = self.generateName(sender, self.instId)

+        pp_key = (msg, sender) if isinstance(msg, PrePrepare) else None
+        if pp_key and (pp_key not in self.pre_prepare_tss):
+            self.pre_prepare_tss[pp_key] = self.utc_epoch


I think it's better to use self.get_time_for_3pc_batch for better unit tests

plenum/server/replica.py

ashcherbakov · 2019-04-09T09:13:49Z

plenum/server/replica.py

@@ -2650,7 +2672,7 @@ def process_requested_prepare(self, prepare: Prepare, sender: str):
    def process_requested_commit(self, commit: Commit, sender: str):
        return self._process_requested_three_phase_msg(commit, sender, self.requested_commits)

-    def is_pre_prepare_time_correct(self, pp: PrePrepare) -> bool:
+    def is_pre_prepare_time_correct(self, pp: PrePrepare, sender: str) -> bool:


It would be great to cover this by unit tests. See, for example, test_replica_freshness.py

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

…-1983-ts-container-for-pp Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

plenum/server/replica.py

ashcherbakov · 2019-04-10T07:44:25Z

plenum/common/messages/node_messages.py

@@ -165,6 +167,16 @@ class PrePrepare(MessageBase):
    )
    typename = PREPREPARE

+    def _post_process(self, input_as_dict: Dict) -> Dict:


Why do we need this?

Yes, to make PrePrepare hashable

ashcherbakov · 2019-04-10T07:47:57Z

plenum/server/replica.py

+        # Dictionary:
+        #   key: Tuple[pp.viewNo, pp.seqNo]
+        #   value: Dict[PrePrepare, Tuple[sender, timestamp]]
+        self.pre_prepare_tss = defaultdict(dict)


Why do we need to have a dict of dict and store pre-prepares there?
I think just a simple Dict[(pp.viewNo, pp.seqNo), timestamp] is enough, isn't it?
If this is because of duplicate PrePrepares for the same 3PC key, then this is invalid PrePrepare, which is validated with PP_CHECK_DUPLICATE raised.

Dict[(pp.viewNo, pp.seqNo), timestamp] won't work since:

the same PrePrepare might come multiple times (e.g. for PP that includes not finalized requests, or missed previous PP or with bad timestamps), but we need to set timestamp only first time

(pp.viewNo, pp.seqNo) might be the same for multiple different PPs (different senders, or single malicious one)

plenum/test/node_catchup_with_3pc/test_pp_obsolescence_check_fail_for_delayed.py

ashcherbakov · 2019-04-10T07:54:11Z

plenum/test/node_catchup_with_3pc/test_pp_obsolescence_check_fail_for_delayed.py

+
+    # Now delayed 3PC messages reach lagging node, so any transactions missed during
+    # catch up can be ordered, ensure that all nodes will have same data after that
+    ensure_all_nodes_have_same_data(looper, txnPoolNodeSet)


We assume that all PPs will be rejected for the lagging node, don't we? If so, how the lagging node may have the same data?

By requesting PP missed for Commits/Prepares

plenum/test/node_catchup_with_3pc/test_pp_obsolescence_check_fail_for_delayed.py

plenum/server/replica.py

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

…-1983-ts-container-for-pp

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

…-1983-ts-container-for-pp

ashcherbakov · 2019-04-10T18:33:57Z

plenum/test/node_request/test_pre_prepare/test_pp_obsolescence.py

+    _fields = {}
+
+
+class FakePrePrepare(FakeMessageBase, PrePrepare):


Why do we need FakePrePrepare here? Why don't use just PrePrepare?

ashcherbakov · 2019-04-10T18:50:05Z

plenum/test/node_request/test_pre_prepare/test_pp_obsolescence_check_fail_for_delayed.py

+    tconf.ACCEPTABLE_DEVIATION_PREPREPARE_SECS = old_value
+
+
+# TODO INDY-2047: this test should actually fail once the bug is fixed


Why the test will fail after INDY-2047 is fixed?

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

…-1983-ts-container-for-pp Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

andkononykhin added 2 commits April 8, 2019 18:39

adds a test for the case

a540ad3

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

adds pp ts tracking and checking

5c301e2

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

ashcherbakov reviewed Apr 9, 2019

View reviewed changes

andkononykhin added 9 commits April 9, 2019 13:05

improves tests

60df4b0

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

improves pp timestamp routine

a4db7d1

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

Merge branch 'master' of github.com:hyperledger/indy-plenum into INDY…

cff4003

…-1983-ts-container-for-pp Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

removes bad merged code

e0c6c85

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

improves collection and cleaning PP timestamps

d27cdb3

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

makes PrePrepare hashable

19870bc

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

fixes test

69e8d1c

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

fixes pp ts management

1d04260

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

adds comment

b760236

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

ashcherbakov reviewed Apr 10, 2019

View reviewed changes

plenum/server/replica.py Outdated Show resolved Hide resolved

andkononykhin added 9 commits April 10, 2019 13:34

optimizes tests

8dfa9c1

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

makes sender a part of key in pp tss dict

75a8619

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

fixes getattr in MessageBase, moves tests

b1d4d31

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

adds unit tests for pp obsolescence logic

cbdc927

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

Merge branch 'master' of github.com:hyperledger/indy-plenum into INDY…

5da79c4

…-1983-ts-container-for-pp

updates comment

bc10d7f

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

fixes a typo

af64d3b

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

fixes tests

0390acc

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

Merge branch 'master' of github.com:hyperledger/indy-plenum into INDY…

92b79e0

…-1983-ts-container-for-pp

ashcherbakov reviewed Apr 10, 2019

View reviewed changes

andkononykhin added 3 commits April 11, 2019 08:51

disables cleanup of additional requests during gc

0d9222e

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

makes flake8 happy

8a00a2e

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

Merge branch 'master' of github.com:hyperledger/indy-plenum into INDY…

35d082d

…-1983-ts-container-for-pp Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

ashcherbakov approved these changes Apr 11, 2019

View reviewed changes

ashcherbakov merged commit 54d2bd3 into hyperledger:master Apr 11, 2019

andkononykhin deleted the INDY-1983-ts-container-for-pp branch April 11, 2019 10:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INDY-1983] Tracking PP first validation time to check their obsolescences #1155

[INDY-1983] Tracking PP first validation time to check their obsolescences #1155

andkononykhin commented Apr 8, 2019

ashcherbakov Apr 9, 2019

andkononykhin Apr 10, 2019

ashcherbakov Apr 9, 2019

andkononykhin Apr 10, 2019

ashcherbakov Apr 10, 2019

andkononykhin Apr 10, 2019

ashcherbakov Apr 10, 2019

andkononykhin Apr 10, 2019

ashcherbakov Apr 10, 2019

andkononykhin Apr 10, 2019

ashcherbakov Apr 10, 2019

ashcherbakov Apr 10, 2019

		_fields = {}


		class FakePrePrepare(FakeMessageBase, PrePrepare):

		tconf.ACCEPTABLE_DEVIATION_PREPREPARE_SECS = old_value


		# TODO INDY-2047: this test should actually fail once the bug is fixed

[INDY-1983] Tracking PP first validation time to check their obsolescences #1155

[INDY-1983] Tracking PP first validation time to check their obsolescences #1155

Conversation

andkononykhin commented Apr 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment