feat: add blob protocol to infra #353
Conversation
View stack outputs
|
vasco-santos
force-pushed
the
feat/add-blob-protocol-to-service
branch
from
a35d822
to
ea4fe9a
Compare
| * @param {string} tableName | ||
| * @returns {AllocationsStorage} | ||
| */ | ||
| export function useAllocationsStorage(dynamoDb, tableName) { |
There was a problem hiding this comment.
This is quite similar with storeTable with few modifications given new columns
| * @param {string} bucketName | ||
| * @returns {BlobsStorage} | ||
| */ | ||
| export function useBlobsStorage(s3, bucketName) { |
There was a problem hiding this comment.
This is quite similar with buckets/car-store with few modifications given new key schema
There was a problem hiding this comment.
Might be worth adding this as code comment actually.
vasco-santos
changed the title
| */ | ||
| async schedule(invocation) { | ||
| const connection = this.getServiceConnection() | ||
| const [res] = await connection.execute(invocation) |
There was a problem hiding this comment.
Should this write a receipt into receipt storage ?
There was a problem hiding this comment.
it is not needed. Sadly this looks then a bit confusing and looking forward to have some kind of receipt storage part of ucanto/scheduler later on. Here this does a HTTP request executing the invocation, which goes through the ucan-invocation-router and is hooked up with the receipt storage: https://github.com/web3-storage/w3infra/blob/main/upload-api/functions/ucan-invocation-router.js#L280
I will add a comment to explain this
upload-api/stores/allocations.js
Outdated
| space, | ||
| multihash: multihash58btc, | ||
| size: blob.size, | ||
| invocation: invocation.toString(), |
There was a problem hiding this comment.
I realize this just implements what has be previously added to upload-api, however sadly I'm only recognizing this here. It is not a blocker, but perhaps a good idea to do in a followup if we agree.
I am realizing that this should probably link to the signed receipt as opposed to invocation. In a way all our stores should simply be indexes over receipts because receipts are signed commitments from us, also they do link back to the corresponding invocation.
Rational here is that anyone with access to table could create valid record like this, however if it was linking to signed receipt they would not be able to without signing key.
There was a problem hiding this comment.
I understand your point and this looks like it make sense. I will create an issue to address this as follow up because I am not sure on how we can do this in practise given this is blob/add invocation CID. And its receipt link would not be possible to compute when we run blob/allocate given it is also dependent on this receipt itself.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah sorry I was not very clear here, I meant that it should be receipt to blob/allocate not to receipt of the blob/add
| * @param {string} bucketName | ||
| * @returns {BlobsStorage} | ||
| */ | ||
| export function useBlobsStorage(s3, bucketName) { |
There was a problem hiding this comment.
Might be worth adding this as code comment actually.
| has: async (multihash) => { | ||
| const encodedMultihash = base58btc.encode(multihash) | ||
| const cmd = new HeadObjectCommand({ | ||
| Key: `${encodedMultihash}/${encodedMultihash}.blob`, |
There was a problem hiding this comment.
Do you recall reason why had same named dir and file within it ? I am pretty sure there was one, but I can't seem to recall what it was. If you do could you please add code comment mentioning it so future us can be grateful
There was a problem hiding this comment.
yes, it is related to how Bucket rate limiting happens (at least in S3), where rate limits are per path, rather than entire bucket. With this, we protect ourselves from rate limiting by having folder names as hashes with unique content inside
upload-api/stores/blobs.js
Outdated
| ChecksumSHA256: checksum, | ||
| ContentLength: size, | ||
| }) | ||
| const expiresIn = 60 * 60 * 24 // 1 day |
There was a problem hiding this comment.
Nit: Should we expose this as an optional argument ? Seems like something we may want to be able to change in some cases.
There was a problem hiding this comment.
yes, good call! I will expose this!
Per storacha/w3infra#353 (comment) renaming this to cause so that it is transparent for the infra side what this is: i.e. can be invocation or receipt
vasco-santos
force-pushed
the
feat/add-blob-protocol-to-service
branch
from
f4b15c6
to
dc6f646
Compare
Adds integration tests for blob implementation started on #353 . This test aims to test all the main flows that MUST happen once a Blob is written: - [x] Blob is written and stored on target - [x] Blob is written via `w3up-client` - [x] Blob index is stored on user space - [x] Blob is retrievable from Roundabout - [x] Blob blocks are retrievable from Hoverbord - [x] Blob is retrievable by its root from w3link - [x] Blob makes its way into Filecoin pipeline after a `filecoin/offer` In addition, `store-add` tests are added independently to make sure old clients keep operation for writes + reads! Follow ups: - [ ] @vasco-santos to create an issue to create ENV VARs for the testing names added like `w3s.link` hostname
This PR creates stores and wires up new
upload-apirunningblob/add,web3.storage/blob/allocate,web3.storage/blob/acceptanducan/concludecapabilities. Tests are also imported fromupload-apiimplementation and run here.As agreed on storacha/w3up#1343 , there won't be any deduping between old world and new world. Therefore, we have new
allocationstable, and use different key schema incarpark. We are writing blobs keyed asbase58btcas previously discussed as${base58btcEncodedMultihash}/${base58btcEncodedMultihash}.blob. I added.blobsuffix but I am happy to other suggestions. Depending on how we progress with the reads side, we can consider creating a new bucket to fully isolate new content?The
receiptsandtasksstorage end up being more complicated as they need to follow https://github.com/web3-storage/w3infra/blob/main/docs/ucan-invocation-stream.md#buckets, and is essentially the same as what happens on https://github.com/web3-storage/w3infra/blob/main/upload-api/ucan-invocation.js#L66 but at a different level as this is a proactive write of tasks and receipts.Integration tests are in progress in branch https://github.com/web3-storage/w3infra/tree/feat/add-blob-protocol-to-service-integration-test. They currently run in my local dev setup, and have full functionality from
blob/add, followed by HTTP PUT to bucket anducan/concludecontaining the receipt of the put. This makes scheduledblob/acceptrun and a receipt with a location claim being available. You can see the tests in branch, on filetest/blob.test.js. I want to only add them here once we can rely on the client to do the manual hack I did here