Skip to content
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

feat: compute piece for uploaded cars #228

Merged
merged 4 commits into from
Sep 18, 2023

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Sep 14, 2023

This is the first of a series of PRs to integrate w3up with w3filecoin.

This PR adds Filecoin Stack, where CARPARK bucket is hooked up with a lambda to compute Piece CIDs via Event Bus events. A queue is in the middle to keep the flow steady. Once a PR is computed, its details are written into a DynamoDB table. This will have all the information users might need to ask, in order to then ask Deal Tracker for information for a given deal. In other words, it maps piece, link (Car CID), aggregate and inclusionProof (TBD what it actually is, is it a CID for a stored file in a bucket?).

Also integration tests were added.

In a follow up PR, a event will be hooked up from DynamoDB table insert to trigger segment/add capability from Aggregator together with writing a claim to equivalency claims.

TODO:

filecoin/index.js Outdated Show resolved Hide resolved
@seed-deploy
Copy link

seed-deploy bot commented Sep 14, 2023

View stack outputs

@vasco-santos vasco-santos force-pushed the feat/compute-piece-for-uploaded-cars branch from 48d8bb6 to b4ae896 Compare September 14, 2023 16:53
@seed-deploy seed-deploy bot temporarily deployed to pr228 September 14, 2023 16:53 Inactive
@vasco-santos vasco-santos marked this pull request as ready for review September 14, 2023 16:59
@@ -21,7 +22,7 @@ export default function (app) {
app.setDefaultFunctionProps({
runtime: 'nodejs16.x',
environment: {
NODE_OPTIONS: "--enable-source-maps",
NODE_OPTIONS: "--enable-source-maps --experimental-fetch",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, lets get that upgrade done soon!

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, I left a few nits. Please can we also publish an equivalency claim?

)
export class DatabaseOperationFailed extends Error {
get reason() {
return this.message
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Why not just use the message property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It follows the same pattern as ucanto does https://github.com/web3-storage/ucanto/blob/c8999a59852b61549d163532a83bac62290b629d/packages/core/src/result.js#L63 for failures, so that the types of failures from handlers of ucanto server are all well aligned. So, not strictly necessary, but consistent

filecoin/functions/piece-cid-compute.js Show resolved Hide resolved
filecoin/index.js Outdated Show resolved Hide resolved
filecoin/index.js Outdated Show resolved Hide resolved
"@aws-sdk/client-sqs": "^3.226.0",
"@sentry/serverless": "^7.22.0",
"@web3-storage/data-segment": "^3.0.1",
"fr32-sha2-256-trunc254-padded-binary-tree-multihash": "github:web3-storage/fr32-sha2-256-trunc254-padded-binary-tree-multihash",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be npm now?

filecoin/tables/piece.js Outdated Show resolved Hide resolved
})
})

test('computes piece cid from a CAR file in the bucket', async t => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test('computes piece cid from a CAR file in the bucket', async t => {
test('computes piece CID from a CAR file in the bucket', async t => {

@@ -21,7 +22,7 @@ export default function (app) {
app.setDefaultFunctionProps({
runtime: 'nodejs16.x',
environment: {
NODE_OPTIONS: "--enable-source-maps",
NODE_OPTIONS: "--enable-source-maps --experimental-fetch",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, lets get that upgrade done soon!

test/helpers/table.js Outdated Show resolved Hide resolved
minTimeout: 1000,
retries: 100
})
} catch {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd let this throw - if we don't find after 100 retries it ain't coming.

@vasco-santos
Copy link
Contributor Author

This LGTM, I left a few nits. Please can we also publish an equivalency claim?

Yes, it is mentioned in main comment and will come as follow up PR

@vasco-santos
Copy link
Contributor Author

CI for integration tests is failing given resources were destroyed for kinesis for this cloudformation

@vasco-santos vasco-santos merged commit 0ec8078 into main Sep 18, 2023
1 check passed
@vasco-santos vasco-santos deleted the feat/compute-piece-for-uploaded-cars branch September 18, 2023 16:37
vasco-santos added a commit that referenced this pull request Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants