-
Notifications
You must be signed in to change notification settings - Fork 516
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
Read only ledger flag #364
Read only ledger flag #364
Conversation
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #364 +/- ##
==========================================
- Coverage 89.17% 89.14% -0.04%
==========================================
Files 236 236
Lines 11218 11222 +4
==========================================
Hits 10004 10004
- Misses 1214 1218 +4 |
Is the last point about the DID update endpoint a joke? That's the one that caused the problem. I guess I have to look at the code. |
I guess it is not a joke. I don't understand. Let's go to a call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of this change is to block all writes so that writes are only done when expected.
The call to update the did endpoint is made from within the agent: It's not invoked from the controller. So the options are to:
Or:
|
Let's discuss in person. I think it should be refactor as I think the
danger of inadvertent writing is too great - screwing up the wallet or
unanticipated costs for writing. Perhaps that's over-reacting, but I think
the better design is to have an explicit application that handles the
writes and the operational app work with what it has.
I also don't understand why the agent would write something without the
controller telling it, but we can talk about that. I guess it's
a byproduct of a start up option, which is arguably the controller telling
the agent to do it. But again, that can be separated into the two modes of
operation.
…On Thu, Feb 6, 2020 at 12:18 PM Ian Costanzo ***@***.***> wrote:
The call to update the did endpoint is made from within the agent:
https://github.com/hyperledger/aries-cloudagent-python/blob/master/aries_cloudagent/config/ledger.py#L78
It's not invoked from the controller.
So the options are to:
- ignore this call when in "read only" mode
Or:
- refactor the code so the controller invokes this call directly
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#364>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHYRQT3736NVAM7BOGGSDLRBRWC5ANCNFSM4KRCYM6A>
.
--
Stephen Curran
Principal, Cloud Compass Computing, Inc. (C3I)
Technical Governance Board Member - Sovrin Foundation (sovrin.org)
*Schedule a Meeting: **https://calendly.com/swcurran
<https://calendly.com/swcurran>*
|
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Set "read_only" to False in "provision" mode and to True in "start" mode. Made "endpoint" parameter available in "provision" mode. Added a "--dev-provision" flag to allow some provision activities in "start" mode to support local development (specifically to allow creation of schemas and credential definitions). |
Done |
Done, "aca-py start" will block all ledger updates, unless "--dev-provision" is also provided
Signed-off-by: Ian Costanzo <[email protected]>
Add a parameter to set the ledger to "read only"
Agent will throw an exception for any attempted ledger write operations
EXCEPT a "DID update endpoint" will be silently ignored