feat: store equals for both content and equals multihash #23

olizilla · 2023-09-18T12:39:21Z

We want to return equals claims when quired with either the content cid or the equals cid, so store 2 records in dynamo for equals claims.

assert/equals claims provide 2 CIDs. The content CID and the equals CID that we claim the content CID is equivelant to. (e.g. different hash fn for same bytes)

The dynamo records are { content: "base58btc-multihash", claim: CID }. The actual bytes of the claim are stored in s3, only once per claim.

For equals claims we store an additional record that sets the content to the multihash of the provided equals CID.

Alternatively we could have stored an equals property on dynamo records for equals claims, and added a global secondary index for that field, and updated the query to check both the content field and the equals field (and the secondary index) for every query, but this would incur additonal read costs for every query. It is assumed that storing a extra tuple of [string,cid] per equals claim will be cheap, and avoids a larger refactor of the claims db.

License: MIT

We want to return equals claims when quired with either the content cid or the equals cid, so store 2 records in dynamo for equals claims. `assert/equals` claims provide 2 CIDs. The `content` CID and the `equals` CID that we claim the `content` CID is equivelant to. (e.g. different hash fn for same bytes) The dynamo records are `{ content: "base58btc-multihash", claim: CID }`. The actual bytes of the claim are stored in s3, only once per claim. For equals claims we store an additional record that sets the `content` to the multihash of the provided `equals` CID. **Alternatively** we could have stored an `equals` property on dynamo records for equals claims, and added a global secondary index for that field, and updated the query to check both the content field and the equals field (and the secondary index) for every query, but this would incur additonal read costs for *every* query. It is assumed that storing a extra tuple of [string,cid] per equals claim will be cheap, and avoids a larger refactor of the claims db. License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

olizilla · 2023-09-18T12:40:28Z

I'm looking at the walk api now to see how that needs to change to deal with bi-directional equals claims now.

olizilla · 2023-09-18T12:50:51Z

Walk api is not critical to this work. We will be querying by content cid only. There is a more general problem of "the walk api needs to guard against loops" that should be tackled before we go live with that api. Issue raised #24

olizilla · 2023-09-18T13:02:32Z

packages/lambda/src/lib/store/index.js

-  async put ({ claim, bytes, content, expiration }) {
-    const hasExpiration = expiration && isFinite(expiration)
-    const cidstr = claim.toString(base32)
+  async put (claim) {


factored out the s3 put and the db upsert so i can call upsertClaims twice for equals claims.

olizilla · 2023-09-18T13:03:56Z

packages/core/src/server/api.ts


 export interface Claim {
  claim: Link
  bytes: Uint8Array
  content: MultihashDigest
-  expiration?: number
+  expiration?: number,
+  value: AnyAssertCap


passing the full capability object through to the claims store allows us to inspect the capability name, e.g. assert/equals and special case it, and we also get the nb property on the capability, so we can extract the equals cid.

also it has a pleasing symmetry with the mutliformats block api which has the bytes and the decoded value of a block.

a CIDv0 can be equivalent to other CIDs, so fix the types License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

olizilla · 2023-09-18T13:42:32Z

packages/core/src/client/api.ts

@@ -69,9 +69,9 @@ export interface RelationPartInclusion {
 /** A claim that the same data is referred to by another CID and/or multihash */
 export interface EqualsClaim extends ContentClaim<typeof Assert.equals.can> {
  /** Any CID e.g a CAR CID */
-  readonly content: Link
+  readonly content: UnknownLink


type fix... a v0 CID can be equivalent to another CID, so the types should allow UnknownLink here.

vasco-santos

Code changes look good to me! Thanks for putting it together

Defer the decision if this is desirable, or if to be desirable client should perform 2 calls instead fo @alanshaw as I am not familiar with the protocol details.

Out of scope, but it would be great to consider for this project to improve error handling in the same lines that we do in w3filecoin / ucanto, where we type and move around the potential errors, instead of relying on errors to be thrown

vasco-santos · 2023-09-18T13:33:44Z

packages/lambda/src/lib/store/index.js

+    return dynamoClient.send(cmd)
+  }, {
+    minTimeout: 100,
+    onFailedAttempt: err => console.warn(`failed DynamoDB update for: ${mh}`, err)


Should we keep the error as it was, or perhaps extend it with mh as well?

Perhaps would make easier to see where the error comes if it logs the cidstr like the other error

vasco-santos · 2023-09-18T13:37:47Z

packages/lambda/src/lib/store/index.js

+      }
+      // also add the claim with the `equals` cid as the `content` cid,
+      // so we can provide the equivalent claim for look ups for either.
+      await upsertClaim({ ...claim, content: equivalent }, this.#table)


I wonder if we should add any metadata to this? Otherwise, might get complex if we in the future want to change something and not know what was added on the other way. Thoughts?

I'm in favour of adding the capability name to the dynamo record, so we could in future we could let user ask for "just assert/equals or whichever flavour they want, and it has the side benefit of letting us reduce the number of records we'd have to explore to back out this choice, without adding a explicit "this is one of those reverse claims"

seed-deploy · 2023-09-18T14:48:57Z

View stack outputs

pr23-content-claims-API

Name	Value
cloudfrontUrl	https://d7he2noehuruz.cloudfront.net
functionName	pr23-content-claims-API-fn5FF616E3-ExfIQhVIW029
functionUrl	https://3wcnzzystlujd4y2wz53syisyi0lfrxr.lambda-url.us-east-2.on.aws/
url	https://pr23.claims.web3.storage

pr23-content-claims-Bucket
pr23-content-claims-DB

alanshaw

Looks great! I added a few non-blocking feedbacks.

packages/lambda/src/lib/store/index.js

Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

🤖 I have created a release *beep* *boop* --- ## [3.1.0](content-claims-v3.0.1...content-claims-v3.1.0) (2023-09-19) ### Features * add `assert/equals` ([#22](#22)) ([bddd948](bddd948)) * ensure claim API is CID version agnostic ([#18](#18)) ([1690c1f](1690c1f)) * store equals for both content and equals multihash ([#23](#23)) ([715fcd5](715fcd5)) ### Bug Fixes * key content on multihash not CID ([#21](#21)) ([7e737a7](7e737a7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

seed-deploy bot had a problem deploying to pr23 September 18, 2023 12:39 Failure

olizilla commented Sep 18, 2023

View reviewed changes

olizilla requested review from vasco-santos and alanshaw September 18, 2023 13:07

fix: type for equals should be UnknownLink

069116e

a CIDv0 can be equivalent to other CIDs, so fix the types License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot had a problem deploying to pr23 September 18, 2023 13:41 Failure

olizilla commented Sep 18, 2023

View reviewed changes

vasco-santos approved these changes Sep 18, 2023

View reviewed changes

seed-deploy bot temporarily deployed to pr23 September 18, 2023 14:39 Inactive

alanshaw approved these changes Sep 18, 2023

View reviewed changes

packages/lambda/src/lib/store/index.js Outdated Show resolved Hide resolved

packages/lambda/src/lib/store/index.js Outdated Show resolved Hide resolved

packages/lambda/src/lib/store/index.js Outdated Show resolved Hide resolved

chore: apply suggestions from code review

a7561ad

Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>

seed-deploy bot temporarily deployed to pr23 September 19, 2023 10:23 Inactive

fix: upsert equivalent claim in parallel

5dc4967

License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>

seed-deploy bot temporarily deployed to pr23 September 19, 2023 10:55 Inactive

olizilla merged commit 715fcd5 into main Sep 19, 2023
3 checks passed

olizilla deleted the return-equals-for-content branch September 19, 2023 11:10

github-actions bot mentioned this pull request Sep 19, 2023

chore(main): release content-claims 3.1.0 #19

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: store equals for both content and equals multihash #23

feat: store equals for both content and equals multihash #23

olizilla commented Sep 18, 2023

olizilla commented Sep 18, 2023

olizilla commented Sep 18, 2023

olizilla Sep 18, 2023

olizilla Sep 18, 2023

olizilla Sep 18, 2023

olizilla Sep 18, 2023

vasco-santos left a comment

vasco-santos Sep 18, 2023

vasco-santos Sep 18, 2023

olizilla Sep 19, 2023

olizilla Sep 19, 2023

seed-deploy bot commented Sep 18, 2023

alanshaw left a comment

feat: store equals for both content and equals multihash #23

feat: store equals for both content and equals multihash #23

Conversation

olizilla commented Sep 18, 2023

olizilla commented Sep 18, 2023

olizilla commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vasco-santos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seed-deploy bot commented Sep 18, 2023

alanshaw left a comment

Choose a reason for hiding this comment