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

get cap cid and log it, for later retrieval of cacao #1153

Merged
merged 7 commits into from
Sep 12, 2023

Conversation

gvelez17
Copy link
Contributor

@gvelez17 gvelez17 commented Aug 31, 2023

Log cap CID - #PLAT-1567

Description

We need cacaos in order to track app usage, and the cap cid will allow us to retrieve cacaos.

How Has This Been Tested?

Describe the tests that you ran to verify your changes. Provide instructions for reproduction.

  • [x ] added unit tests to cover new lines
  • [x ] manually inspected the carfile blobs produced by js-ceramic

Definition of Done

Before submitting this PR, please make sure:

  • [x ] The work addresses the description and outcomes in the issue
  • [ x] I have added relevant tests for new or updated functionality
  • [x ] My code follows conventions, is well commented, and easy to understand
  • [ x] My code builds and tests pass without any errors or warnings
  • [x ] I have tagged the relevant reviewers
  • I have updated the READMEs of affected packages
  • I have made corresponding changes to the documentation
  • The changes have been communicated to interested parties

References:

Please list relevant documentation (e.g. tech specs, articles, related work etc.) relevant to this change, and note if the documentation has been updated.

@gvelez17 gvelez17 changed the title sergey changes to start extract cacao and propagate thru get cap cid and log it, for later retrieval of cacao Sep 1, 2023
Copy link
Member

@oed oed left a comment

Choose a reason for hiding this comment

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

It looks like this PR get's the CACAO from the genesis commit if present. There could also be another CACAO that's attached to the commit that is being anchored (root.tip). This CACAO would tell us the application that made the anchor request.

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 1, 2023

It looks like this PR get's the CACAO from the genesis commit if present. There could also be another CACAO that's attached to the commit that is being anchored (root.tip). This CACAO would tell us the application that made the anchor request.

Oh this is super helpful - 2 question:

is it possible both root.tip and the signature cid url will both be present and different, or can we just fallback to check both for a single value? Is there one we shoudl check first?

what is the meaning of the root.tip one? Is that always generated on genesis commits? I was meaning to ask you if we would always get a cacao if there was not an update after the genesis commit

Note this is the url, not the actual cacao.  will retrieve async from ipfs
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 90.00% and project coverage change: -0.01% ⚠️

Comparison is base (bbe02b3) 81.90% compared to head (937923d) 81.90%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1153      +/-   ##
===========================================
- Coverage    81.90%   81.90%   -0.01%     
===========================================
  Files           46       46              
  Lines         1697     1702       +5     
  Branches       258      260       +2     
===========================================
+ Hits          1390     1394       +4     
- Misses         307      308       +1     
Files Changed Coverage Δ
src/ancillary/anchor-request-params-parser.ts 89.36% <88.88%> (-1.34%) ⬇️
src/services/request-service.ts 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 2, 2023

ok, to cover the uncovered line, i need to write a test that covers this https://app.codecov.io/gh/ceramicnetwork/ceramic-anchor-service/commit/937923d0b2191606c5f30004467312b7c6d14434/blob/src/ancillary/anchor-request-params-parser.ts#L115

which is derived from genesisJWS.signatures

so i looked in the tests, and it turns out, that the test labeled

const CAR_FILE_REQUEST_EXAMPLE_SIGNED_GENESIS = mockRequest({

doesn't actually have any signatures

(determined this with this tool https://github.com/3box/runbooks-and-oncall/blob/main/CACAOTools/min_decode.mjs using the blob from the test)

so now, the question is, how to construct a carfile that DOES have a signature?

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 2, 2023

oh! also if it was present, it might be in the data, because js-ceramic would have put it there! but our test cases don't show it, bc they are not really signed, i think


    if (StreamUtils.isSignedCommitContainer(data)) {
        const { jws, linkedBlock, cacaoBlock } = data
        // if cacao is present, put it into ipfs dag
        if (cacaoBlock) {
          const decodedProtectedHeader = base64urlToJSON(data.jws.signatures[0].protected)
          const capCID = CID.parse(decodedProtectedHeader.cap.replace('ipfs://', ''))
          carFile.blocks.put(new CarBlock(capCID, cacaoBlock))
          restrictBlockSize(cacaoBlock, capCID)
          this.dagNodeCache.set(capCID.toString(), carFile.get(capCID))
        }

@oed
Copy link
Member

oed commented Sep 2, 2023

is it possible both root.tip and the signature cid url will both be present and different, or can we just fallback to check both for a single value? Is there one we shoudl check first?

what is the meaning of the root.tip one? Is that always generated on genesis commits? I was meaning to ask you if we would always get a cacao if there was not an update after the genesis commit

I believe that root.tip is either the CID of the genesisCommit if that's the only commit in the stream, otherwise it's the most recent signedCommit.

@oed
Copy link
Member

oed commented Sep 2, 2023

I believe there are four cases as outlined below (maybe @ukstv can confirm this)

Untitled Diagram drawio(4)

  1. a single genesis commit that isn't signed
  2. a single genesis commit that is signed, here there could be a CACAO present
  3. a signed commit that points to an unsigned genesis commit, here there could be a CACAO present
  4. a signed commit that points to a signed genesis commit
    In this example there might be a CACAO present for the signed commit, however I believe that even if the genesis commit reference a CACAO it won't be present in the CAR file.

reference code

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 5, 2023

mmm @oed @ukstv it would be great to have test data blobs for each of those cases! it was taking me a long time to generate one, i will try some more today, maybe if i set up local ceramic to talk to cas and see the data going across

Is there a clear way to make js-ceramic do each case?

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 5, 2023

or do they all look the same inside CAS? sorry i am a little confused tying this diagram to different cases in the code, i will look some more

I guess it just means - anytime we have JWS commit, we can check for cacao on the protected header

Does it mean that the cacao is always only in the protected header?

Also don't worry that the actual cacao is not there, i am planning to make async lambda that retrieves from ipfs in the metrics db

@stbrody
Copy link
Contributor

stbrody commented Sep 5, 2023

@oed I don't understand cases 3 and 4 that you outlined above. What's the difference between a SignedCommit and a GenesisCommit for a stream that has never been updated past the genesis state?

@oed
Copy link
Member

oed commented Sep 5, 2023

@oed I don't understand cases 3 and 4 that you outlined above. What's the difference between a SignedCommit and a GenesisCommit for a stream that has never been updated past the genesis state?

For a stream that has never been updated past genesis state only cases 1 and 2 apply. Cases 3 and 4 describe streams that have been updated >= 1 times!

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 5, 2023

@stbrody what does it take to run a local js-ceramic and cause it to sign with a cacao? Will that happen if i just use the composedb libraries to generate some model documents? I want to see if it actually puts it into the carfile or if its just in the signatures in the header.

@oed
Copy link
Member

oed commented Sep 6, 2023

@gvelez17 you need to use the did-session package with the ethereum auth provider and you'll get a Cacao.

@gvelez17
Copy link
Contributor Author

gvelez17 commented Sep 8, 2023

ok, i constructed a client that is creating streams successfully locally, but js-ceramic is never making the anchor request when i am running locally? https://github.com/ceramicnetwork/js-ceramic/blob/develop/packages/cli/src/ceramic-daemon.ts#L576 is never reached.

We are out of time, i will just submit this for approval without the test i would like to have.

Thx to Sergey for a demonstration how to make js-ceramic generate them, now we have test coverage.

@gvelez17 gvelez17 marked this pull request as ready for review September 8, 2023 06:05
expect(params.capCID).toEqual(FAKE_CAPCID)
})

test('throws error on invalid car file', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this does not throw an error. the validator returns the junk

Here is invalid validation: {"_tag":"Left","left":[{"trail":[{"key":"","type":{"name":"RequestAnchorParamsV2"},"actual":{"0":67,"1":114,"2":101,"3":97,"4":116,"5":101,"6":100,"7":66,"8":121,"9":67,"10":104,"11":97,"12":116,"13":71,"14":80,"15":84,"16":52,"17":89,"18":111,"19":117,"20":99,"21":97,"22":110,"23":85,"24":115,"25":101,"26":84,"27":104,"28":105,"29":115,"30":83,"31":116,"32":114,"33":105,"34":110,"35":103}}]}]}

@oed @ukstv shouldn't the validator throw an error on this junk?

Copy link
Contributor Author

@gvelez17 gvelez17 Sep 11, 2023

Choose a reason for hiding this comment

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

i can separate out this part into a separate pr since it is not a change affected by the capCID

moved this test here https://github.com/ceramicnetwork/ceramic-anchor-service/pull/1162/files

@gvelez17 gvelez17 requested a review from oed September 11, 2023 19:37
@gvelez17
Copy link
Contributor Author

It looks like this PR get's the CACAO from the genesis commit if present. There could also be another CACAO that's attached to the commit that is being anchored (root.tip). This CACAO would tell us the application that made the anchor request.

i already have that one in the logs, so will use both in retrieving the actual CACAO from ipfs. this is to add the other possible source.

@gvelez17 gvelez17 requested a review from ukstv September 11, 2023 19:38
* Revert "move invalid carfile tests to separate PR"

This reverts commit d82ee7b.

* invalid car file produces isLeft; cannot decode throws error

* ok an invalid carfile has isLeft, but what about some random string?

* remove duplicate coverage test
@gvelez17 gvelez17 merged commit 07c3509 into develop Sep 12, 2023
@gvelez17 gvelez17 deleted the feat/extract-cacao branch September 12, 2023 12:58
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.

5 participants