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

chore(kubo)::> Generic DagProvider.go added #847

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

PsychoPunkSage
Copy link

related to issue:kubo#10386 and PR:kubo#10704. Refer this

Problem

  • Reprovider strategy currently ignores MFS content
  • Each provider implementation has its own DAG traversal logic
  • Need a generic way to announce content from any DAG structure

Solution

Added NewDAGProvider that:

  • Takes a root CID and traverses its entire DAG
  • Uses existing fetcherhelpers.BlockAll for efficient traversal
  • Works with any DAG structure (MFS, IPLD, etc.)
  • Integrates with existing provider prioritization

@PsychoPunkSage PsychoPunkSage requested a review from a team as a code owner February 16, 2025 04:26
Copy link

welcome bot commented Feb 16, 2025

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions
    and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the
    rest) and in its best form. Follow the code contribution
    guidelines

    if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on
    any missing things and potentially assigning a reviewer for high
    priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

Copy link
Contributor

@hsanjuan hsanjuan left a comment

Choose a reason for hiding this comment

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

Some comments. Please add some testing as well.

You have utils to generate dags at github.com/ipfs/boxo/ipld/merkledag/test. Verify outCh produces the expected Cids.

@PsychoPunkSage
Copy link
Author

Hi @hsanjuan
Did some changes and wrote a test.... BUT Its failing.... Can't seem to get where I'm missing the point

Test Logs

=== RUN   TestNewDAGProvider
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:163: Created DAG with root QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv and 7 expected CIDs
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 1: QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 2: QmU5KjgAuvhC79E7jMVBWBg29Aoprdk6jL7qxrXKw1gCTU
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 3: QmYGarcCmpcMYbteSQtrkU6xogQZSFa3d1SqjKq19RR9Qm
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 4: QmNRGfMaSjNcjtyS56JrZBEU5QcGtfViWWG8V9pVqgVpmT
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 5: Qmb28WBcFQ7EPZP38pCLsEhAjxwDDsi14My9LaPNJzVfqu
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 6: QmWiwiH33PJBvTSDS81UnjpzBzdiHhUGWmzrfKPpdbHMy6
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:165: Expected CID 7: QmTCzUiekoo67MDJqenUXr5ioND7EQ5YjRQzG4ARmVqVzo
2025/02/18 21:56:47 Starting DAG traversal with root: QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv
2025/02/18 21:56:47 Starting BlockMatchingOfType
2025/02/18 21:56:47 Processing CID QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv (1 processed)
2025/02/18 21:56:47 Added CID to set: QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv
2025/02/18 21:56:47 Completed DAG traversal, processed 1 CIDs
2025/02/18 21:56:47 Completed BlockAll traversal
2025/02/18 21:56:47 Sent CID to output: QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:179: Collected CID: QmR1TEGLLewb5Arp6z9QWTzUNgxVvAAJ5vMpnkDQJ9CFiv
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:183: Collected 1 CIDs
    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:186:
                Error Trace:    /home/psychopunk_sage/dev/OpenSource/boxo/provider/dagprovider_test.go:186
                Error:          Not equal:
                                expected: 7
                                actual  : 1
                Test:           TestNewDAGProvider
                Messages:       number of collected CIDs should match the expected number
--- FAIL: TestNewDAGProvider (0.00s)
FAIL
FAIL    github.com/ipfs/boxo/provider   0.005s

@PsychoPunkSage
Copy link
Author

hi @hsanjuan
Can you please review this PR??

The DAG provider creates a KeyChanFunc for all the CIDs found traversing the
merkledag from given root.

It can be used to provide specific DAGs (i.e. MFS) from their root.
@hsanjuan hsanjuan self-assigned this Feb 27, 2025
Copy link
Contributor

@hsanjuan hsanjuan left a comment

Choose a reason for hiding this comment

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

Hello, I have fixed your code, which could be done much simpler. I think the work involved for this PR was too complex for you and I suggest you try easier issues first. When I read your code, I wonder if you are using an AI assistant as well to write it, or parts of it at least?

@gammazero Can you have a look, since reviewing would be mostly reviewing myself.

This adds a SkipNotFound option to the FetcherConfig skips branch traversal when the blockservice returns ipld.NotFound.

The result is that the fetcher is able to traverse dags in offline mode.
@hsanjuan hsanjuan dismissed their stale review February 28, 2025 15:35

i fixed it

@hsanjuan hsanjuan requested a review from gammazero March 6, 2025 13:59
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.

3 participants