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

ADR009: Non-Interactive Default Rules for Reduced Padding #1003

Merged
merged 12 commits into from
Dec 13, 2022

Conversation

nashqueue
Copy link
Member

@nashqueue nashqueue commented Nov 14, 2022

This PR is regarding this Question.

rendered

nashqueue pushed a commit to nashqueue/celestia-app that referenced this pull request Nov 15, 2022
@MSevey MSevey removed their request for review November 15, 2022 17:59
@MSevey
Copy link
Member

MSevey commented Nov 15, 2022

Deferring to @liamsi and @adlerjohn

rootulp
rootulp previously approved these changes Nov 16, 2022
Copy link
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

I don't have any blocking feedback, it's all optional to make the document read easier.

I plan on doing another review later this week to inspect the math closer but don't want to block the PR on my re-review.

docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
docs/architecture/adr-009-proof-size-analysis.md Outdated Show resolved Hide resolved
@nashqueue
Copy link
Member Author

nashqueue commented Nov 18, 2022

Thinks i need to adjust the calculation:

  • Add parity shares as blue nodes to the calculation
  • Lower the size of the nodes above the Row roots as it is not a nmt
  • Last Row has more blue nodes that it needs
  • Update diagrams and result

@rootulp rootulp changed the title ADR009: New Non-Interactive Default Rules for Reduced Padding ADR009: Non-Interactive Default Rules for Reduced Padding Nov 22, 2022
Copy link
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

No blocking feedback, still LGTM

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

Nice! Really great analysis 👍 Left a few suggestions and questions. I'd defer to others before merging, but if this ADR is going stale after a month or so, I am fine merging provided we keep it as Proposed status.

I still need to think about the tradeoffs a bit more, but fwiw I'm leaning towards being in favor of this proposal.

If we are only thinking about bandwidth costs of super light clients, and we are only thinking about 134MB blocks (square size 512) for the foreseeable future. Then in the worst possible case we would only increase super light clients bandwidth by about 11 GB per year. (roughly 2,102,400 blocks per year (4 per minute), max block size each block, and worst size of rollup block each block)

However, if we went with this proposal have a more efficient padding, then we would either have a smaller average square size or be able to post more data. If we have a smaller average square size, then we reduce the bandwidth costs of minimized light clients because they gossip a smaller average DAH.

@nashqueue
Copy link
Member Author

However, if we went with this proposal have a more efficient padding, then we would either have a smaller average square size or be able to post more data. If we have a smaller average square size, then we reduce the bandwidth costs of minimized light clients because they gossip a smaller average DAH.

Unfortunately this might be false considering the Jevons_paradox. It would be the same as making Blockspace bigger to decrease congestion, but more block space could in turn create more demand creating the same congestion.

@evan-forbes
Copy link
Member

what are the todos left on this @nashqueue before we merge?

@nashqueue
Copy link
Member Author

what are the todos left on this @nashqueue before we merge?

Nothing to add ship it.

@evan-forbes evan-forbes merged commit f9adbd7 into celestiaorg:main Dec 13, 2022
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.

6 participants