fix: multiple space diffs #456
Conversation
…ability instead of BlobAllocate - Duplicate usage events occurred because the blob/allocate receipt was added to the UCAN stream multiple times. - The blob/allocate receipt was embedded within the blob/add receipt. - The UCAN invocation router extracted receipts from agent messages, adding blob/allocate to the stream initially and again when processing blob/add. - Resolution: Changed the capability used to track space usage deltas from BlobAllocate to BlobAccept to avoid duplicate events.
View stack outputs
|
| resource = message.value.att[0].nb?.space | ||
| size = message.out.ok.size | ||
| size = message.value.att[0].nb?.blob.size |
There was a problem hiding this comment.
Ah, and this works because value points to the original invocation for blob accept, which does include the size.
There was a problem hiding this comment.
The only problem with this approach is that it doesn't take into account whether the blob is stored in the space already or not. The receipt for blob/allocatewill have size: 0 when the blob already exists in the space.
It's probably better than the current situation though...
There was a problem hiding this comment.
If size: 0, we are not adding it to the diff table.
There was a problem hiding this comment.
Yes, but I'm saying we'll never receive that. We will add a new record to the space diff table every time the user attempts to store bafyxyz, no matter how many times.
e.g. bafyxyz is 100 bytes
Client store bafyxyz -> insert space diff -> space size = 100
Client store bafyxyz -> insert space diff -> space size = 200
Client store bafyxyz -> insert space diff -> space size = 300
...
but the behaviour we want is:
Client store bafyxyz -> insert space diff -> space size = 100
Client store bafyxyz -> no space diff insert -> space size = 100
There was a problem hiding this comment.
@hannahhoward WDYT shall we merge and release this? IMHO it's better than the status quo but still needs more work.
There was a problem hiding this comment.
If the blob already exists, we return success for blob/allocate with a size of 0 and out.ok.address set to null. However, if this occurs, we immediately return the receipt for http/put, and if it has succeeded, we execute blob/accept, leading to two receipts in the stream.
There was a problem hiding this comment.
I think in that case we should just not generate those receipts early.
There was a problem hiding this comment.
Would love to catch up on this one - if it doesn't come up at sprint planning or we don't have enough time to get into it I'd love to sync briefly after that if y'all are free!
There was a problem hiding this comment.
OK - in our chat this morning we decided to ship this change since it will significantly decrease the impact of the duplicate diff issue, and to deprecate the ucan stream entirely as part of the upload-service work - @alanshaw should we create one more item under that upload service epic for this work? I'm happy to go file that and fill in some basic details if so...
There was a problem hiding this comment.
I summarized the plan moving forward here: storacha/RFC#39
Co-authored-by: ash <[email protected]>
Description:
This PR addresses the issue storacha/project-tracking#199, where duplicate space diff entries were being created when uploading a file using
space/blob/add.Problem:
Context: When the UCAN invocation router receives a request, it executes the invocation and writes the receipts to the stream, which is then processed by
billing/ucan-stream.jsto populate the diff table.Unlike
store/add,space/blob/addspawns three additional invocations:web3.store/blob/allocate,http/put, andweb3.store/blob/acceptbefore finalizing.Due to the way receipts are extracted from agent messages—which are a collection of invocations and receipts without a clear hierarchy—this structure causes the receipt from
web3.store/blob/allocateto be sent to the stream twice:blob/allocatereceipt is embedded within theblob/addreceipt.Solution:
Change the capability used to track space usage deltas from
BlobAllocatetoBlobAccept, asBlobAcceptdoes not appear multiple times.