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(share): return fixed data size in blockstore #3634

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

cristaloleg
Copy link
Contributor

Dummy solution from #3630 . We return a fixed size for any parameters. Always return a max size of 2 possible values: inner or leaf node size.

Fixes #3630

@cristaloleg cristaloleg added the kind:fix Attached to bug-fixing PRs label Aug 8, 2024
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

One last thing that make me feel uneasy is that we always return the same size even when the real data is less. I spend another hour looking for a solution, but there is no deterministic way.

So we can either return size of a leaf or a node. Leaf size is bigger than node, so we always report more than we actually serve to Bitswap and I worry about implications of lying to bitswap. Also, if we lie, whats better, to over report with leaves or to underreport with nodes.

share/eds/blockstore.go Outdated Show resolved Hide resolved
@cristaloleg cristaloleg force-pushed the share/return-fixed-size branch from 7626857 to d99c6b3 Compare August 9, 2024 10:20
@walldiss walldiss enabled auto-merge (squash) August 20, 2024 16:10
@walldiss walldiss merged commit 9fd66a5 into main Aug 20, 2024
25 of 28 checks passed
@walldiss walldiss deleted the share/return-fixed-size branch August 20, 2024 16:20
walldiss pushed a commit to walldiss/celestia-node that referenced this pull request Aug 21, 2024
Wondertan pushed a commit that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:fix Attached to bug-fixing PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FN/BN CPU BBQ
3 participants