[INDY-2073] Dynamic validation for client requests taa acceptance data #1195

andkononykhin · 2019-05-07T16:10:12Z

No description provided.

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

…-2073-request-taa-validation

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

…-2073-request-taa-validation

…alidation

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

…alidation

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

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

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

andkononykhin · 2019-05-15T15:06:03Z

test this please

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

ashcherbakov · 2019-05-16T09:11:54Z

plenum/common/ledger_manager.py

        )

        self._node_leecher.register_ledger(ledger_id)

+    def ledger_info(self, lid: int) -> Optional[LedgerInfo]:
+        try:
+            return self.ledgerRegistry[lid]


Why don't we use self.ledgerRegistry,get(lid) instead?

thanks, updated

ashcherbakov · 2019-05-16T09:30:13Z

plenum/server/node.py

+            )
+            return
+
+        config_req_handler = self.ledger_to_req_handler.get(CONFIG_LEDGER_ID)


I think it's better to use get_req_handler(ledger_id=CONFIG_LEDGER_ID)

agreed, updated

ashcherbakov · 2019-05-16T09:32:05Z

plenum/server/node.py

+        if taa_data is not None:
+            taa, taa_seq_no, taa_txn_time = taa_data
+
+        if not taa:


In case of empty TAA, only the test is empty, but the version is not. So, not taa check is not enough here.

ashcherbakov · 2019-05-16T09:34:29Z

plenum/server/node.py

+        #        "Txn Author Agreement acceptance mechanism list is not defined"
+        #    ) # TODO test
+
+        if not request.taaAcceptance:


I think it makes sense to do this check before 2 previous ones (AML check and TAA_digest check), right after we decide if we need a TAA. This can save some time going into states (for AML and digest)

ashcherbakov · 2019-05-16T09:36:46Z

plenum/server/node.py

+            return
+
+        # TODO not (None or empty)  ???
+        taa_digest = config_req_handler.get_taa_digest()  # TODO test that digest match taa


We've already get TAA data for the 'latest' key. When doing this, we've already got the digest as well (because we have 'latest' -> 'digest' and 'digest' -> 'data'). So, there is no need to go to the state again.
I think get_taa_data needs to return the digest as well.

ashcherbakov · 2019-05-16T09:41:41Z

plenum/server/node.py

+        taa_aml = {}
+        # taa_aml = config_req_handler.get_taa_aml()
+        # if not taa_aml:
+        #    raise TaaAmlNotSetError(


I think we can raise a LogicError here, since there is a dynamic validation that TAA can be set only after there is non-empty AML present. So, according to this logic, TAA without an AML can not happen.

ashcherbakov · 2019-05-16T09:55:41Z

plenum/server/node.py

+            self.config.TXN_AUTHOR_AGREEMENT_ACCEPANCE_TIME_BEFORE_TAA
+        )
+        ts_higest = (
+            self.now +


It looks like we need to use the time of the primary's PrePrepare this request is present in instead of the local node's time.

yes, updated

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

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

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

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

ashcherbakov · 2019-05-17T09:27:45Z

plenum/common/exceptions.py

@@ -18,6 +22,14 @@ class NodeError(Exception):
    pass


+class PoolError(Exception):


Do we need this Error?

Not so much

ashcherbakov · 2019-05-17T09:36:57Z

plenum/server/config_req_handler.py

    def get_taa_data(self, digest: Optional[str] = None,
                     version: Optional[str] = None,
-                     isCommitted: bool = True) -> Optional[Dict]:
+                     isCommitted: bool = True) -> Tuple[Optional[Dict], Optional[str]]:
+        data = None
        if digest is None:


If digest is not None, then None data will be returned. Is it intended?

right, fixed

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

ashcherbakov · 2019-05-17T10:52:57Z

plenum/server/node.py

+        if taa_data is not None:
+            (taa, taa_seq_no, taa_txn_time), taa_digest = taa_data
+
+        if taa is None:


After a discussion with Andrey, we decided that no TAA must be present in a request if TAA is not set/disabled.

ashcherbakov · 2019-05-17T12:43:36Z

plenum/test/txn_author_agreement/acceptance/test_taa_acceptance_validation.py

+    ):
+        validate_taa_acceptance(domain_req, pp_time)
+
+


Please add a test for checking that PrePrepare time is used for validation, not node's local time.

ashcherbakov · 2019-05-17T12:45:55Z

plenum/test/txn_author_agreement/acceptance/test_taa_disabled.py

+
+
+@pytest.mark.taa_acceptance_missed
+def test_taa_acceptance_missed_during_disabled_taa(


Please add a test where TAA is present, but with an empty test.

ashcherbakov

Approving, but please make sure that we do the following in the next PR:

Please add a (unit) test where TAA is present, but with an empty test.
Please add a (unit) test for checking that PrePrepare time is used for validation, not node's local time.
Please make sure (a fix and a (unit) test) that no TAA must be present in a request if TAA is not set/disabled.
integration tests with libindy

adds initial tests for taa acceptance validation

b470224

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

andkononykhin changed the title ~~[INDY-2073] Dynamic validation for client taa acceptance data [WIP][ci skip]~~ [INDY-2073] Dynamic validation for client requests taa acceptance data [WIP][ci skip] May 7, 2019

andkononykhin added 16 commits May 8, 2019 14:12

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

deca8e5

…-2073-request-taa-validation

Merge branch 'pr-1196' into INDY-2073-request-taa-validation

ef7fbd4

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

moves taa acceptance tests to new location

f079dcd

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

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

52167bc

…-2073-request-taa-validation

Merge branch 'INDY-2066-taa-ts-in-state' into INDY-2073-request-taa-v…

e5f7a07

…alidation

adds API for taa accpetance necessity per ledger

a439cbb

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

configures node ledger regarding taa acceptance

7c66f2c

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

Merge branch 'INDY-2066-taa-ts-in-state' into INDY-2073-request-taa-v…

2744f3a

…alidation

improves fixtures and helpers, fixes tests

b25a4ef

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

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

70aedba

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

added forgotten file

d0cafe5

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

updates test

2ef3de9

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

fixes tests

a1b4918

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

adds ledger_info API to LedgerManager

d67e304

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

adds fixture to get taa

f283540

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

adds validation implementation, fixes tests partly

7e56e05

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

andkononykhin changed the title ~~[INDY-2073] Dynamic validation for client requests taa acceptance data [WIP][ci skip]~~ [INDY-2073] Dynamic validation for client requests taa acceptance data [WIP] May 15, 2019

makes flake8 happy

66b5470

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

ashcherbakov reviewed May 16, 2019

View reviewed changes

andkononykhin added 7 commits May 16, 2019 13:56

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

1208c69

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

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

1de1b3e

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

improves taa validation, fixes tests

6a5d660

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

adds AML check to validation, updates test

77ded5b

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

optimizes validation logic

97fd9cc

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

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

f1018d4

…-2073-request-taa-validation Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

improves tests fictures

e7fbf64

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

fixes imports

828fb8a

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

ashcherbakov reviewed May 17, 2019

View reviewed changes

fixes get_taa_data logic and test

406957e

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

andkononykhin changed the title ~~[INDY-2073] Dynamic validation for client requests taa acceptance data [WIP]~~ [INDY-2073] Dynamic validation for client requests taa acceptance data May 17, 2019

ashcherbakov reviewed May 17, 2019

View reviewed changes

ashcherbakov approved these changes May 17, 2019

View reviewed changes

ashcherbakov merged commit 6ba94d3 into hyperledger:master May 17, 2019

andkononykhin deleted the INDY-2073-request-taa-validation branch May 17, 2019 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INDY-2073] Dynamic validation for client requests taa acceptance data #1195

[INDY-2073] Dynamic validation for client requests taa acceptance data #1195

andkononykhin commented May 7, 2019

andkononykhin commented May 15, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 16, 2019

andkononykhin May 16, 2019

ashcherbakov May 17, 2019

andkononykhin May 17, 2019

ashcherbakov May 17, 2019

andkononykhin May 17, 2019

ashcherbakov May 17, 2019

ashcherbakov May 17, 2019

ashcherbakov May 17, 2019

ashcherbakov left a comment

		@@ -18,6 +22,14 @@ class NodeError(Exception):
		pass


		class PoolError(Exception):



		@pytest.mark.taa_acceptance_missed
		def test_taa_acceptance_missed_during_disabled_taa(

[INDY-2073] Dynamic validation for client requests taa acceptance data #1195

[INDY-2073] Dynamic validation for client requests taa acceptance data #1195

Conversation

andkononykhin commented May 7, 2019

andkononykhin commented May 15, 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

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

ashcherbakov left a comment

Choose a reason for hiding this comment