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

service/state: Add SubmitPayForData endpoint #619

Closed
wants to merge 2 commits into from

Conversation

renaynay
Copy link
Member

@renaynay renaynay commented Apr 13, 2022

This PR introduces a SubmitPayForData endpoint.

@adlerjohn adlerjohn added kind:feat Attached to feature PRs area:core_and_app Relationship with Core node and Celestia-App labels Apr 13, 2022
@renaynay renaynay added the area:state Related to fetching state and state execution label Apr 13, 2022
@renaynay renaynay changed the title TODO BuildPayForData service/state: Add SubmitPayForData endpoint Apr 23, 2022
@renaynay renaynay force-pushed the pfm-endpoint branch 3 times, most recently from 370ebc6 to 3533355 Compare April 25, 2022 11:25
@renaynay
Copy link
Member Author

renaynay commented Apr 25, 2022

Initially @liamsi suggested that I add a parameter to the SubmitPFD endpoint called maxSize so that the user could specify the (max share size || max num shares)-- clarify this for me @liamsi they're willing to pay for and if the message size exceeds that, then the request would fail.

After speaking w/ @evan-forbes, we decided that we should just leave this parameter out for now until they've explored non-interactive defaults further in celestia-app. Currently, it will only add something like max 240 bytes to the message, but with non interactive defaults, that could balloon to something much larger in which case it would be useful.

Another point is that we want to remove this code entirely from celestia-node and have the canonical PFD constructor / submitter methods to exist inside celestia-app instead. Meaning, whenever we do add non interactive defaults, we can introduce this param in celestia-app instead.

WDYT @liamsi (and maybe @adlerjohn)

Ref celestiaorg/celestia-core#455

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core_and_app Relationship with Core node and Celestia-App area:state Related to fetching state and state execution kind:feat Attached to feature PRs
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants