Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[INDY-2154] Cleanup after req handlers integration #276

Merged
merged 16 commits into from
Jul 9, 2019

Conversation

lampkin-diet
Copy link
Contributor

No description provided.

@lampkin-diet lampkin-diet changed the title [skip ci][ci skip][INDY-2154] Cleanup after req handlers integration [INDY-2154] Cleanup after req handlers integration Jul 7, 2019
skhoroshavin
skhoroshavin previously approved these changes Jul 7, 2019
Signed-off-by: Andrew Nikitin <[email protected]>
skhoroshavin
skhoroshavin previously approved these changes Jul 8, 2019
@@ -48,7 +49,7 @@ def test_state_proof(public_minting, looper, # noqa
encoded = {}
outputs = res[OUTPUTS]
for out in outputs:
state_key = TokenReqHandler.create_state_key(out["address"], out["seqNo"])
state_key = create_state_key(out["address"], out["seqNo"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use a method from GetUtxoHandler?

node.write_manager.register_batch_handler(h)
# TODO: Additional functionality to request_manager ^^^

domain_fee_batch_handler = DomainFeeBatchHandler(node.db_manager, fees_tracker)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we need to put it before Audit Handler since Audit ledger tracks ledger root hashes and sizes for all ledger (two ledgers are changed if we order a domain txn with fees)

KitHat
KitHat previously requested changes Jul 8, 2019
@@ -34,6 +34,10 @@ def static_validation(self, request: Request):
request.reqId,
error)

@staticmethod
def create_state_key(address: str, seq_no: int) -> bytes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some consistency here.
We have already defined this method in https://github.com/sovrin-foundation/token-plugin/blob/master/sovtoken/sovtoken/request_handlers/token_utils.py#L30
We should either use the one in token_utils.py either delete it and use only the one here.
By the way, this is a utility method and don't see any reason to keep it in this class.

Andrew Nikitin added 4 commits July 9, 2019 10:09
@ashcherbakov ashcherbakov dismissed KitHat’s stale review July 9, 2019 08:12

Approved by two other maintainers

@lampkin-diet lampkin-diet merged commit c3d3d22 into sovrin-foundation:master Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants