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

Fluffy: Implement poke in state network #2750

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bhartnett
Copy link
Contributor

@bhartnett bhartnett commented Oct 18, 2024

Poke in the state network is feasible by passing in the parent offer when looking up content. The fetched state content is then combined with the parent offer to build the offer which is used in the poke.

I'm going to suggest that poke is added to the state network specs because I can't see any reason for it to be disabled after implementing this change.

I've also added poke to the stateGetContent JSON-RPC method by supporting an optional parentContent parameter which should contain the parent offer which is required in order to enable poke.

@bhartnett bhartnett requested a review from kdeme October 18, 2024 07:52
@bhartnett bhartnett marked this pull request as draft October 18, 2024 12:00
@bhartnett
Copy link
Contributor Author

Leaving this as a draft PR until the specs are updated

Copy link
Contributor

@kdeme kdeme left a comment

Choose a reason for hiding this comment

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

lgtm but would wait with merging until accepted in specs first.

Comment on lines 86 to 88
rpcServer.rpc("portal_stateGetContent") do(
contentKey: string, parentContent: Opt[string]
) -> ContentInfo:
Copy link
Contributor

Choose a reason for hiding this comment

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

For a user this API does complicate things. Feels like the "health" of the network should not be pushed towards the UX of user API. But perhaps its ok as an Opt...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I agree and for this reason I didn't propose changing the JSON-RPC spec to add this in my other PR. I'm planning to revert this part but I'll wait to see what happens with the spec changes first.

@bhartnett bhartnett marked this pull request as ready for review November 26, 2024 07:11
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