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-2313: Require ratification_ts to create TAA #1437

Merged
merged 10 commits into from
Dec 24, 2019

Conversation

skhoroshavin
Copy link
Member

Signed-off-by: Sergey Khoroshavin sergey.khoroshavin@dsr-corporation.com

Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@skhoroshavin skhoroshavin changed the title INDY-2313: Require ratification_ts to create TAA [WIP] INDY-2313: Require ratification_ts to create TAA Dec 23, 2019
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@skhoroshavin skhoroshavin changed the title [WIP] INDY-2313: Require ratification_ts to create TAA INDY-2313: Require ratification_ts to create TAA Dec 23, 2019
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@lgtm-com
Copy link

lgtm-com bot commented Dec 23, 2019

This pull request introduces 2 alerts when merging e98d6d0 into f2d582b - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method
  • 1 for Unused import

Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
ashcherbakov
ashcherbakov previously approved these changes Dec 23, 2019
"Cannot create transaction author agreement without a '{}' field."
.format(TXN_AUTHOR_AGREEMENT_RATIFICATION_TS))

if ratification_ts > req_pp_time:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we consider some Delta (for a possible clock skew) here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. We might need it, but it wasn't in requirements. Also, I think this shouldn't be a problem since production use case most probably will look like:

  • create TAA txn with ratification time current or earlier
  • pass TAA txn to trustees so they sign it (this process most probably will take at least several minutes)
  • submit signed TAA txn - with ratification date already well in the past

Also, when author of TAA txn is trustee and he wants to send txn immediately he can put ratification date into the past himself (which is also done by integration tests).

@lgtm-com
Copy link

lgtm-com bot commented Dec 23, 2019

This pull request introduces 1 alert when merging 1a89ca4 into f2d582b - view on LGTM.com

new alerts:

  • 1 for Unused import

Toktar
Toktar previously approved these changes Dec 23, 2019
ashcherbakov
ashcherbakov previously approved these changes Dec 23, 2019
@lgtm-com
Copy link

lgtm-com bot commented Dec 23, 2019

This pull request introduces 1 alert when merging 0aaed82 into 021a81e - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Dec 24, 2019

This pull request introduces 1 alert when merging f716aae into 4350709 - view on LGTM.com

new alerts:

  • 1 for Unused import

skhoroshavin and others added 2 commits December 24, 2019 13:29
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@lgtm-com
Copy link

lgtm-com bot commented Dec 24, 2019

This pull request introduces 1 alert when merging 72acf37 into f7492a0 - view on LGTM.com

new alerts:

  • 1 for Unused import

Sergey Khoroshavin added 2 commits December 24, 2019 14:13
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@skhoroshavin skhoroshavin merged commit 160853a into hyperledger:master Dec 24, 2019
@skhoroshavin skhoroshavin deleted the indy-2313 branch December 24, 2019 12:51
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.

None yet

3 participants