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

Add information about state witness size limits to NEP 509 #547

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

jancionear
Copy link
Contributor

No description provided.

@jancionear jancionear requested a review from wacban June 13, 2024 19:20
@jancionear jancionear requested a review from a team as a code owner June 13, 2024 19:20
Copy link
Contributor

@wacban wacban left a comment

Choose a reason for hiding this comment

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

LGTM

neps/nep-0509.md Outdated
@@ -128,6 +128,51 @@ TODO: This is essentially going to be describing the exact structure of `ChunkSt

The section should return to the examples given in the previous section, and explain more fully how the detailed proposal makes those examples work.]

## State witness size limits

Some limits are introduced to keep the size of `ChunkStateWitness` reasonable.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would rephrase to something like: "A number of new limits will be introduced in order to ..."


Some limits are introduced to keep the size of `ChunkStateWitness` reasonable.
`ChunkStateWitness` contains all the incoming transactions and receipts that will be processed during chunk application and in theory a single receipt could be tens of megabatytes in size. Distributing a `ChunkStateWitness` this large would be troublesome, so we limit the size and number of transactions, receipts, etc. The limits aim to keep the total uncompressed size of `ChunkStateWitness` under 16MiB.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also mention that this is a breaking change but that we don't predict it to affect more than x% of transactions and receipts? Can you also mention that the contract developers can work around the limits by reducing how much data is accessed in a single call (am I getting this right?)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a section about breaking changes. I prepared a list of transactions larger than 1.5MiB. Contract developers can take a look at this list and see if their contract will be affected by the new limits.

neps/nep-0509.md Outdated

### Missing chunks

When a chunk is mising on a shard, this shard will receive receipts from more than one block height. This could lead to large `source_receipt_proofs` so a mechanism is added to reduce the impact. If there are two or more missing chunks in a row,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: When a shard is missing some chunks, the following chunk on that shard will receive receipts from multiple blocks.

Comment on lines +147 to +148
* `combined_transactions_size_limit - 2 MiB`
* Hard limit on total size of transactions from this and previous chunk. `ChunkStateWitness` contains transactions from two chunks, this limit applies to the sum of their sizes.
Copy link
Member

Choose a reason for hiding this comment

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

Is this calldata? How does this work in practice? Does this mean that If we have 2 separate transactions at max_transaction_size, we have to wait since they cannot be included consecutively, since there is 3mb of calldata? What happens to the transactions that are larger than that, do they get dropped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If someone submits two transactions at max_transaction_size then one of them will be included and the other one will stay in the transaction pool.

It would look like this:

  • User submits 2 transactions, each of them at max_transaction_size
  • Height 1 - transaction 1 is included
  • Height 2 - no transaction is included because the previous chunk had 1.5MiB of transactions
  • Height 3 - transaction 2 is included, as the last chunk didn't contain any transactions, so it fits under the limit.

## State witness size limits

Some limits are introduced to keep the size of `ChunkStateWitness` reasonable.
`ChunkStateWitness` contains all the incoming transactions and receipts that will be processed during chunk application and in theory a single receipt could be tens of megabatytes in size. Distributing a `ChunkStateWitness` this large would be troublesome, so we limit the size and number of transactions, receipts, etc. The limits aim to keep the total uncompressed size of `ChunkStateWitness` under 16MiB.
Copy link

@serrrfirat serrrfirat Jun 14, 2024

Choose a reason for hiding this comment

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

Do we have any hard evidence that can back this claim? I think it would be great to see the limits of the current system in this NEP. It would greatly increase the justification of the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We observed missing chunks when witness size reached ~16MB. @shreyan-gupta then implemented a new distribution mechanism, but I'm not sure if there have been any tests of the throughput with the new mechanism.

I believe we need some limits, for example I was able to create a 300TGas receipt which produced 24MB of storage proof. If there are 3 such receipts in a chunk, then that would add up to 72MB of state witness size. Apart from that we could have a lot of incoming receipts, there is no receipt size limit, but assuming that each receipt is 5MiB in size, and we received receipts from all 6 shards, that would add up to 30MB in incoming receipts. This means that without limits witness size could easily reach 100MB of size. We had problems with 16MB, I'm sure 100MB would be even worse.

* On most block heights a shard isn't allowed to send receipts larger than 100 KiB to another shard.
* `outgoing_receipts_big_size_limit - 4.5 MiB`
* On every block height there's one special "allowed shard" which is allowed to send larger receipts, up to 4.5 MiB in total.
* A receiving shard will receive receipts from `num_shards - 1` shards using the usual limit and one shard using the big limit.

Choose a reason for hiding this comment

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

How is this shard determined? Is it going to be randomly picked every block? Is it going to be a gateway shard that allows for big receipts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the allowed_shard from congestion control NEP. I'll add a paragraph about it.

@jancionear jancionear requested a review from wacban June 14, 2024 11:53
@jancionear jancionear merged commit ca02c29 into near:state-validation Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: APPROVED FIXES
Development

Successfully merging this pull request may close these issues.

4 participants