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

Fix/claim fallback cid #85

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

hannahhoward
Copy link
Member

@hannahhoward hannahhoward commented Jan 11, 2025

Goals

Fix issues with #82

Implementation

My identity CID approach has a weakness -- the actual CID used to look up claims is the link for the root block, not cid of the whole claim serialized to CAR.

This PR converts to using root links as the identity cid, rather than the whole serialized claim. Fortunately, this is roughly equivalent since the synthesized claim has only one block.

This means that a fallback using an identity CID will work all the way through the query process.

You can see an alternate approach in the commit history continues to use the identity CID of the serialzied CAR but modifies the legacy claims mapper so that it passes on this CID correctly. Ultimately I decided this approach was more "correct" and less messy, though it means some slightly odd reconstruction of delegations to maintain the identity CID.

query before

> go run ./cmd query -u https://hannah.indexer.storacha.network baguqeera2tvekmqau6baz36o5cudqiekmcb2boqspofdjip3b3ib2hn6k6qq
2025-01-10T17:03:48.214-0800	FATAL	cmd	cmd/main.go:179	querying service: http request failed, status: 500 Internal Server Error, message: processing query: failure response fetching claim. URL: https://prod-content-claims-bucket-claimsv1bucketefd46802-1mqz6d8o7xw8.s3.us-west-2.amazonaws.com/bafyreie4t46e4jhj5cxuhxedghgvackmz5qjfgbnu5jubgfpn5t3xh5f54/bafyreie4t46e4jhj5cxuhxedghgvackmz5qjfgbnu5jubgfpn5t3xh5f54.car, status: 403 Forbidden, message: <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AccessDenied</Code><Message>Access Denied</Message><RequestId>AAQA544AX7932BM3</RequestId><HostId>CdB5O9RC4GzyU1UTWqCCX+t5m6u6XeoNaVKB/alTLLsSbPV1LEo21u03bEilxaWwx5EFmsp2FWQ=</HostId></Error>

query after

> go run ./cmd query -u https://hannah.indexer.storacha.network baguqeera2tvekmqau6baz36o5cudqiekmcb2boqspofdjip3b3ib2hn6k6qq

Query:
  Hashes (1):
    zQmcfnLAk4i1bZETqNde8qL6fjFA5CNikbNkdK6qQLdj67a

Results:
  Claims (2):
    bafyreibkvewseedfog6lbbkaijzbo2ggbz2nwna2frntxsxdvcceiwfcoy
      Type:
        assert/index
      Content:
        baguqeera2tvekmqau6baz36o5cudqiekmcb2boqspofdjip3b3ib2hn6k6qq
      Index:
        bagbaieradicy56yjedke4kaqw2aq34scek4wzoytw37suh34bfi5fdskubgq
    bafyreig4f3n73zzv5plhngchcejkanmvodjvery5ztyxwd4tjwzudbkpke
      Type:
        assert/location
      Content:
        zQmQ6EE3NqfJTaC5DtxUCLbtQ9bVoVGss8PAqP51MLdJ6uA
      Locations:
        https://carpark-prod-0.r2.w3s.link/zQmQ6EE3NqfJTaC5DtxUCLbtQ9bVoVGss8PAqP51MLdJ6uA/zQmQ6EE3NqfJTaC5DtxUCLbtQ9bVoVGss8PAqP51MLdJ6uA.blob
      Range: 0-379

  Indexes (1):
    bagbaiera23wlwcndsnbnp6g4f4a53obzwrjlygai4pmxqodfzgjyd4n5kanq
      Content:
        baguqeera2tvekmqau6baz36o5cudqiekmcb2boqspofdjip3b3ib2hn6k6qq
      Shards (1):
        zQmSQ26wcoCYcdMpwUbMkaZNfow3Y8B3iHjCjNkCqwkSAGT
          Slices (2):
            zQmSQ26wcoCYcdMpwUbMkaZNfow3Y8B3iHjCjNkCqwkSAGT @ 0-186
            zQmcfnLAk4i1bZETqNde8qL6fjFA5CNikbNkdK6qQLdj67a @ 98-186

Copy link

codecov bot commented Jan 11, 2025

Codecov Report

Attention: Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/service/contentclaims/identitycidfinder.go 33.33% 4 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
pkg/aws/bucketfallbackmapper.go 87.23% <100.00%> (+5.98%) ⬆️
pkg/service/contentclaims/identitycidfinder.go 52.63% <33.33%> (-22.37%) ⬇️

... and 1 file with indirect coverage changes

@hannahhoward hannahhoward force-pushed the fix/claim-fallback-cid branch from 5af8a60 to 3d57c83 Compare January 12, 2025 05:44
@hannahhoward
Copy link
Member Author

upon further investigation, there's an issue with the final approach -- a "Data Integrity Error" when you can .Data() on the claim, because the blocks in UCAN are required to be CBOR encoded. I've reverted to my initial approach of just passing the claim CID

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.

1 participant