ADR 013: Non-Interactive Default Rules for Zero Padding #1161

nashqueue · 2023-01-03T10:44:56Z

Overview

This ADR looks at the possibility of reducing intra-blob padding to close to zero in most cases.

rendered

closes rollkit/rollkit#640

Todo:

Add Graphics

Checklist

New and updated code has appropriate documentation
New and updated code has new and/or updated testing
Required CI checks are passing
Visual proof for any user facing features like CLI or documentation updates
Linked issues closed with keywords

docs/architecture/adr012-non-interactive-default-rules-for-zero-padding.md

rootulp

mostly nits, thanks for the diagrams!

rootulp · 2023-01-05T18:41:21Z

docs/architecture/adr013-non-interactive-default-rules-for-zero-padding.md

+
+![Worst Case Padding In Blob Size Range](./assets/worst-case-padding-in-blob-size-range.png)
+
+This means that you get more padding for fever data in small blobs. This is not ideal as we want to have as little padding as possible. As padding is not being paid for there is no incentive to use larger blobs.


[typo]: "for fever data"

Apologies for no suggestion. I'm not sure what it meant to say.

I rephrased: This means small blobs generate more possible padding in comparison to the data they provide. This is not ideal as we want to have as little padding as possible. As padding is not being paid for there is no incentive to use larger blobs.

docs/architecture/adr013-non-interactive-default-rules-for-zero-padding.md

nashqueue · 2023-01-06T14:12:42Z

Not sure why this housekeeping workflow is breaking

MSevey · 2023-01-06T15:12:09Z

Not sure why this housekeeping workflow is breaking

seems like a bug with forks. I'm working on it. celestiaorg/.github#19

docs/architecture/adr013-non-interactive-default-rules-for-zero-padding.md

evan-forbes

Overall I really like this proposal!! It makes a lot of sense.

Soon we will have multiple blobs per a single PFB. We might want to also want to include some discussion of how this proposal would affect those as well. If there are many small blobs, then might need to use the same threshold mechanism used for large blocks.

[kinda orthogonal discussion]
I believe there was some discussion during the onsite of incorporating resource pricing into a similar proposal, and it might work here as well. Provided we have precise insight into the actual cost to the network each of these options bring, then we can change the cost of gas accordingly. For example, if your blob can have a suitably small inclusion proof while also not taking any padding, then we might want to decrease the amount of gas used for that blob.

nashqueue · 2023-01-11T12:55:55Z

Soon we will have multiple blobs per a single PFB.

What are the specs/ideas for ordering those blobs? What does the commitment of this PFB look like?

evan-forbes · 2023-01-14T00:29:42Z

What does the commitment of this PFB look like?

Currently, the PR has a merkle tree over all of the commitment, but as discussed in #1154 and #1231, we will revert that and simply include each share commitment in the PFB. Originally we decided to move writing the ADR until after the feature freeze, but we might have to start it before that, as this topic requires more thought and buy-in

MSevey · 2023-01-23T18:55:29Z

docs/architecture/adr013-non-interactive-default-rules-for-zero-padding.md

+
+The picture below shows the difference between the old and new non-interactive default rules in a square of size 8 and a threshold of 8.
+
+![Blob Alignment Comparison](./assets/blob-alignment-comparison.png)


I might be missing something but can you elaborate on how this works with the commitments that are independent of square size?

For example, the orange blob would appear to me to have different commitments in the two examples, and so I'd assume it would have a different commitment based on what other data is include in the block.

Or is the intent of this ADR only to focus on reducing padding to the absolute minimum, and not necessarily have padding independent commitments.

It is conseues breaking as it changes the non-interactive default rules. The 2 pictures have 2 different NI-Rules. One before and one after. If you decide on one NI rule it can be either independent of square size or not. In this case both are independent of square size. My goal was to reduce padding and it just happened to almost eliminate padding.

evan-forbes

continuing from #1161 (review)

I think the remaining question is that if there are use cases where we need to verify many blob inclusion proofs, which afaiu is the only case where this would have an effect. State fraud proof producing partial nodes might be that case, where we are increasing the average size for all of the proofs that they have to download, but perhaps they could rely on other nodes to produce fraud message inclusion fraud proofs.

Despite that, I think this would make a great addition and makes a lot of sense! I will defer to other reviewers though, and am looking forward to other's opinions

nashqueue · 2023-02-19T22:03:50Z

Can we merge this as proposed? been idle for a while

evan-forbes · 2023-02-20T13:47:57Z

@nashqueue yeah we can imo. I'd still like to get an accepted tbh, as I think its a really good optimization

rootulp

Great work @nashqueue ! Really informative ADR

musalbas · 2023-02-20T16:37:53Z

Btw I think the status field in these ADRs should be at the top so people can immediately see if they're proposed or accepted etc instead of only realizing it when they get to the bottom.

Addresses #1161 (comment) ## Questions 1. Do we want to make the same change across other celestiaorg repos? 2. Should we consider an org wide ADR template? 1. #1415

evan-forbes · 2023-03-16T15:38:41Z

merging as proposed, but we will have a sync discussion over this during the onsite

gitpoap-bot · 2023-03-16T15:38:53Z

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Celestia Contributor:

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

Addresses celestiaorg/celestia-app#1161 (comment) ## Questions 1. Do we want to make the same change across other celestiaorg repos? 2. Should we consider an org wide ADR template? 1. celestiaorg/celestia-app#1415

nashqueue changed the title ~~inital draft~~ ADR 012: Non-Interactive Default Rules for Zero Padding Jan 3, 2023

nashqueue mentioned this pull request Jan 3, 2023

ADR 013 Zero Padding for an upperBound FraudProofSize rollkit/rollkit#640

Closed

rootulp reviewed Jan 3, 2023

View reviewed changes

nashqueue changed the title ~~ADR 012: Non-Interactive Default Rules for Zero Padding~~ ADR 013: Non-Interactive Default Rules for Zero Padding Jan 5, 2023

rootulp reviewed Jan 5, 2023

View reviewed changes

nashqueue marked this pull request as ready for review January 6, 2023 14:12

nashqueue requested review from liamsi, adlerjohn, MSevey and evan-forbes as code owners January 6, 2023 14:12

cmwaters reviewed Jan 9, 2023

View reviewed changes

docs/architecture/adr013-non-interactive-default-rules-for-zero-padding.md Outdated Show resolved Hide resolved

docs/architecture/adr013-non-interactive-default-rules-for-zero-padding.md Show resolved Hide resolved

evan-forbes reviewed Jan 9, 2023

View reviewed changes

evan-forbes assigned nashqueue Jan 10, 2023

evan-forbes mentioned this pull request Jan 10, 2023

Mandatory non-interactive defaults #935

Closed

MSevey requested a review from a team January 11, 2023 13:08

evan-forbes mentioned this pull request Jan 20, 2023

EPIC: Intra-namespace blob priority #1274

Open

MSevey reviewed Jan 23, 2023

View reviewed changes

evan-forbes approved these changes Jan 24, 2023

View reviewed changes

nashqueue requested review from rootulp and cmwaters February 19, 2023 22:02

nashqueue added 4 commits February 20, 2023 16:28

inital draft

afb55ad

new pictures and resolving commets + new insight

bed3946

resolve nits

1f5ed73

fix rule

6d4dff0

nashqueue force-pushed the new-ni-rules branch from 4d37a03 to 6d4dff0 Compare February 20, 2023 15:28

MSevey requested a review from a team February 20, 2023 15:28

evan-forbes approved these changes Feb 20, 2023

View reviewed changes

evan-forbes requested a review from musalbas February 20, 2023 15:32

rootulp approved these changes Feb 20, 2023

View reviewed changes

rootulp mentioned this pull request Feb 20, 2023

docs: hoist ADR status #1407

Merged

rootulp added a commit that referenced this pull request Feb 22, 2023

docs: hoist ADR status (#1407)

8bd0f99

Addresses #1161 (comment) ## Questions 1. Do we want to make the same change across other celestiaorg repos? 2. Should we consider an org wide ADR template? 1. #1415

evan-forbes merged commit c9dfc54 into celestiaorg:main Mar 16, 2023

evan-forbes mentioned this pull request Mar 30, 2023

Implement dense squares (ADR013) #1572

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADR 013: Non-Interactive Default Rules for Zero Padding #1161

ADR 013: Non-Interactive Default Rules for Zero Padding #1161

nashqueue commented Jan 3, 2023 •

edited

Loading

rootulp left a comment

rootulp Jan 5, 2023

nashqueue Jan 6, 2023 •

edited

Loading

nashqueue commented Jan 6, 2023

MSevey commented Jan 6, 2023

evan-forbes left a comment

nashqueue commented Jan 11, 2023

evan-forbes commented Jan 14, 2023

MSevey Jan 23, 2023

nashqueue Jan 24, 2023

evan-forbes left a comment •

edited

Loading

nashqueue commented Feb 19, 2023

evan-forbes commented Feb 20, 2023 •

edited

Loading

rootulp left a comment

musalbas commented Feb 20, 2023

evan-forbes commented Mar 16, 2023

gitpoap-bot bot commented Mar 16, 2023


		![Worst Case Padding In Blob Size Range](./assets/worst-case-padding-in-blob-size-range.png)

		This means that you get more padding for fever data in small blobs. This is not ideal as we want to have as little padding as possible. As padding is not being paid for there is no incentive to use larger blobs.


		The picture below shows the difference between the old and new non-interactive default rules in a square of size 8 and a threshold of 8.

		![Blob Alignment Comparison](./assets/blob-alignment-comparison.png)

ADR 013: Non-Interactive Default Rules for Zero Padding #1161

ADR 013: Non-Interactive Default Rules for Zero Padding #1161

Conversation

nashqueue commented Jan 3, 2023 • edited Loading

Overview

Checklist

rootulp left a comment

Choose a reason for hiding this comment

rootulp Jan 5, 2023

Choose a reason for hiding this comment

nashqueue Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

nashqueue commented Jan 6, 2023

MSevey commented Jan 6, 2023

evan-forbes left a comment

Choose a reason for hiding this comment

nashqueue commented Jan 11, 2023

evan-forbes commented Jan 14, 2023

MSevey Jan 23, 2023

Choose a reason for hiding this comment

nashqueue Jan 24, 2023

Choose a reason for hiding this comment

evan-forbes left a comment • edited Loading

Choose a reason for hiding this comment

nashqueue commented Feb 19, 2023

evan-forbes commented Feb 20, 2023 • edited Loading

rootulp left a comment

Choose a reason for hiding this comment

musalbas commented Feb 20, 2023

evan-forbes commented Mar 16, 2023

gitpoap-bot bot commented Mar 16, 2023

nashqueue commented Jan 3, 2023 •

edited

Loading

nashqueue Jan 6, 2023 •

edited

Loading

evan-forbes left a comment •

edited

Loading

evan-forbes commented Feb 20, 2023 •

edited

Loading