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

Add doc for freeze ledger #1511

Merged
merged 6 commits into from
Mar 11, 2021

Conversation

adenishchenko
Copy link
Contributor

Signed-off-by: anton.denishchenko [email protected]

Signed-off-by: anton.denishchenko <[email protected]>
@sovbot
Copy link
Contributor

sovbot commented Feb 18, 2021

Can one of the admins verify this patch?

@WadeBarnes WadeBarnes self-requested a review February 19, 2021 13:53
Copy link
Contributor

@Toktar Toktar left a comment

Choose a reason for hiding this comment

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

Please add a chapter "How it affects the system" with a mention of the impact on

  • catchup (frozen ledgers are not catched up, this became possible due to the fact that the LEDGERS_FREEZE transaction is part of the config ledger, which is applied before additional ledgers)
  • freshness (we do not check, we do not send new batches to update frozen ledgers)
  • audit (we continue to write root hashes in audit transactions, but we do not use real ledgers and use data about frozen ledgers from the config state)
  • dynamic validation of transactions (transactions to frozen ledgers are forbidden)

docs/source/transaction_freeze_ledgers.md Outdated Show resolved Hide resolved
docs/source/transaction_freeze_ledgers.md Show resolved Hide resolved
@Toktar
Copy link
Contributor

Toktar commented Feb 20, 2021

And mention remove_ledger.py here, please.

Signed-off-by: anton.denishchenko <[email protected]>
docs/source/transaction_freeze_ledgers.md Outdated Show resolved Hide resolved
Signed-off-by: anton.denishchenko <[email protected]>
@Toktar
Copy link
Contributor

Toktar commented Mar 4, 2021

(ci) test this please

Toktar
Toktar previously approved these changes Mar 4, 2021
@Toktar
Copy link
Contributor

Toktar commented Mar 4, 2021

@WadeBarnes Could you please review the last changes?

* Highlighted the discussion in HIPE 0162
* Grammer corrections
* Changed organization
* Moved discussion of auth map alternative into indy-hipe 0162
* Added some clarifications

Signed-off-by: Richard Esplin <[email protected]>
@esplinr
Copy link
Contributor

esplinr commented Mar 9, 2021

I significantly revised the document so that it reads a lot cleaner. Please confirm that I didn't introduce any inaccuracies @adenishchenko and @Toktar .
Specifically, I didn't understand this phrase: "LEDGERS_FREEZE consists of a list ledger ids, unique version"
I don't see any "unique version" in the code or the documentation in Indy Node, so I removed that reference.

For future readers, here is the link to the PR for documentation in Indy Node:
hyperledger/indy-node#1656

@adenishchenko
Copy link
Contributor Author

@esplinr
Thank you a lot for perfect work!
You right this phrase may confuse and will be better to remove it.
I thought links to HIPE and SIP are useful here, but if you suggest to remove them, I fully agree with you.

@WadeBarnes Could we approve it now?

@Toktar
Copy link
Contributor

Toktar commented Mar 9, 2021

(ci) test this please

Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

One minor change needed. Everything else looks good now.

docs/source/transaction_freeze_ledgers.md Outdated Show resolved Hide resolved
Signed-off-by: anton.denishchenko <[email protected]>
@adenishchenko adenishchenko requested a review from a team as a code owner March 9, 2021 15:47
@WadeBarnes
Copy link
Member

(ci) test this please

@Toktar
Copy link
Contributor

Toktar commented Mar 11, 2021

(ci) test this please

1 similar comment
@WadeBarnes
Copy link
Member

(ci) test this please

@Toktar Toktar merged commit 71abd0f into hyperledger:master Mar 11, 2021
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.

5 participants