Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[GAP-24] Agreement Permissions Management #56
base: master
Are you sure you want to change the base?
[GAP-24] Agreement Permissions Management #56
Changes from all commits
968569c
d5aae4a
0568205
00e80cf
b4c30cd
0809277
0804530
5f4c7e6
41559cb
8081df1
93476f2
b78ac03
05fbf1f
5df0569
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
okay, in this simple model, it's really unclear how the payment process would work in case there is more than one grantee ... who pays? anyone? do we have a "race" to pay? what incentive would grantees have to be the ones to pay? what if multiple grantees sent their
AcceptDebitNote
orAcceptInvoice
messages independently and agree to pay?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.
we abstract from incentives and intents to pay - we only allow for different requestors to pay.
should we raise a "bug" to core team to ensure
AcceptDebitNote
/AcceptInvoice
(as well asReject
) is race-resistant, ie. only a single accept on Invoice/DebitNote should end with 'OK', subsequent attempts should return info "invoice/debitnote already accepted". Must also ensure for this to be exercised on provider-side, as there must be a single-source-of truth and atomicity achieved for Accept operations.how to solve the need to propagate Agreement/Activity related events over the "entitled" Requestors. We need to discuss this with the core team [ACTION - setup workshop with core team].
"Rogue grantee" problem. What options do we have?
0. Do not consider "rogue grantee" problem at all - assume that 'grantees' are reliable
setAgreementPermissions
a. any 'setAgreementPermissions' permission amendment must be controlled by a forum of grantees
b. any permission amendment must be individually controlled by a forum
c. only change in set of grantees must be controlled by a forum
MB: I like 4 :)
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.
"Rogue grantee" problem
grantee
can still Agreement despite consensus being against him.Other options:
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.
Mtg w/Witek on 14.02.:
a. currently yagna (on network level) does not have QoS features, ie. it does not support "retry" on messages sent between sides. This means that if Requestor is offline, the Provider is unable to reliably send TerminateAgreement message (currently one side is unable to Terminate Agreement while the other side is offline). This seems to be a gap from the point of view of "Offline Requestor model"!!!
b. it seems there is a gap in yagna core which currently makes 'attach/detach' impossible
c. agreementId/nodeId propagation:
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.
Small collection of my afterthoughts, which I use in our next session:
Assuming that:
Finally... Requestor should receive similar set of ACL with similar data (but from grantee perspective) - permissions and provider id.
In result, requestor should at least have its own copy of ACL (with provider id), to continue working after kinda-disaster/kinda-offline recovery.
Having only activity id is not enough to revive provider id, without any help of access to supervising entity.
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.
Update on 24.02.:
setAgreementPermissions
validationThere 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.
7.03.2023:
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.
Below is example for defining structure with separate collection of credential providing methods and separate collection of different permission sets.
Because those entries are interconnected and they need to refer from one to each other, using custom names help significantly.
Specific key names and values are provisional and are subject of change.
Relation between
authTypes
andpermissionGroups
could be included inpermissionGroups
(as shown below) or could be moved to analogical structure inauthTypes
.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.
Incorporated the proposed ACL improvements in the GAP draft, with some tweaks (eg. not "authTypes", but rather "attestationTypes", etc.)
We need to discuss the potentially useful attestation types, as we must not include any secrets in the ACL itself!
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.
Notes after 14.03.:
SetAgreementPermissions
)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.
A. Add
attachAgreement
to ACL permissions listB. Add the following aspect:
Provider is expected to actively notify a set of Grantees with Agreement (and respective entity) events. However the list of recipient nodes for events needs to be maintained by the Provider. Following logic is proposed:
attachAgreement
from a granteeId will add this Grantee to the list of event "subscribers"attachAgreement
againyagna
daemon callsattachAgreement
again, in order to get the latest state of the Agreement. (This option is specific to Agreement synchronization and is not a generic mechanism)If the above is available, the event message delivery is guaranteed, and the upper layer (Agreement-specific) does not need to worry about this.
Option 2 is definitely a more substantial change, and requires a broader discussion... it probably should be considered a separate GAP...