FIP-0083: add "client" to verifier-balance event #968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rvagg
requested review from
momack2,
arajasek,
jennijuju,
kaitlin-beegle,
anorth,
raulk,
jsoares and
TippyFlitsUK
as code owners
|
Editor Review: Thank you for the detailed PR description. This is an enhancement for the proposed FIP to be useful for tooling builders and is a well contained change. Approved. |
jennijuju
approved these changes
Mar 20, 2024
14 tasks
anorth
reviewed
Mar 20, 2024
jsoares
approved these changes
Mar 21, 2024
|
lgtm |
aarshkshah1992
added a commit
that referenced
this pull request
Mar 21, 2024
This reverts commit 8b0a604.
This was referenced Mar 21, 2024
14 tasks
8 tasks
rvagg
added a commit
to rvagg/FIPs
that referenced
this pull request
Apr 3, 2024
…coin-project#964 "client" in verifier-balance was merge-resolved out Closes: filecoin-project#971
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Further to #964, this is a proposed change to FIP-0083 that we'd like to slip in to the current network upgrade in order to improve observability for certain observers of the network based on feedback we have received while trying to assist with nv22 integration.
The change here is simply to add a
"client"field to the existingverifier-balanceevent emitted from the verified registry actor. This field will benullfor cases where a verifier's allowance does not change as a result of allocation to a verified client, but will be the actor ID of the client where a client is involved.Ultimately, in discussion, some of us have come to the conclusion that this event was not properly considered when originally implemented and does not allow for straightforward tracking of verifier allowances, and in its current form doesn't help connecting relationships and flows between verifiers and clients. Rather than attempting to augment or fix this event, we will likely propose a new FIP after this network upgrade to replace this event entirely with a new suite of events focused on datacap allowance and allocation flows to and from verifiers. e.g.
add-allowance- emitted when a verifier's allowance is set usingAddVerifier, possibly including a delta and final balance since this method can be used to initialise, add or even decrease an allowance.allocate-datacap- emitted when a verifier allocates datacap to a verified client, including the client (the"client"in this proposal), the amount allocated, and the final balance of the verifier's allowance.remove-verifier- emitted when a verifier is removed from the verified registry, possibly including the final balance of the verifier's allowance.Additional work could be done in the datacap actor to add observability to flows on the client side.
In the meantime, we believe that adding the field in this PR to the existing
verifier-balancefield will provide a lightweight but useful stopgap for the nv22 integration and other observers of the network.Discussion thread specifically for this change is: #754 (comment)