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.

Sign up for GitHub

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

Jump to bottom

Add support block size limit #2133

Closed
xgreenx opened this issue Aug 27, 2024 · 2 comments · Fixed by #2195
Closed

Add support block size limit #2133

xgreenx opened this issue Aug 27, 2024 · 2 comments · Fixed by #2195
Assignees
@rafal-ch
Labels
good first issue Good for newcomers

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Aug 27, 2024

Currently, the block is limited by the gas, and with the 63 gas per byte, the maximum size of the block can be 30000000/63 = 476_190 byte.

But since we want to have a hard limit 128kb or 256kb, we either need to increase the gas per byte, or add a new block size limit variable.

Support the block size limit requires:

  • Add a new block_size_limit field to the consensus parameters.
  • Updating the executor's logic to get transactions based on the remaining size.
  • Update the transaction pool to support the limitation by size.
@xgreenx xgreenx added the good first issue Good for newcomers label Aug 27, 2024
@xgreenx
Copy link
Collaborator Author

xgreenx commented Aug 27, 2024

This feature requires breaking change into the WASM executor since we need to pass the size of the block when fetching transactions from the TxPool. It would be nice to release this breaking change along with #2114 and #1887

@xgreenx xgreenx mentioned this issue Aug 29, 2024
Merged
@rafal-ch rafal-ch self-assigned this Sep 3, 2024
@rafal-ch
Copy link
Contributor

rafal-ch commented Sep 6, 2024

A few comments w.r.t to implementation:

  • new consensus parameter – block_transaction_size_limit
    • not block_size_limit due to what we’re gonna be actually limiting is the set of L2 transactions
  • we’re ok if occasionally the block will get bigger than a limit due to L1 transactions, mint transaction or a header. Hence, if we want block to be limited to roughly 128kb, we coule set the block_transaction_size_limit to 125-126kb
  • size of a transaction = size of the result of canonical serialization of transaction

@acerone85 acerone85 mentioned this issue Sep 9, 2024
Closed
This was referenced Sep 11, 2024
Closed
Merged
Merged
Merged
@xgreenx xgreenx mentioned this issue Sep 15, 2024
Merged
4 tasks
Dentosal added a commit that referenced this issue Sep 17, 2024
@rafal-ch @xgreenx @Dentosal
Add support for ConsensusParametersVersion::V2 (#2188)
0ae006e 
Partial implementation of
#2133.

## Linked Issues/PRs
(FuelLabs/fuel-vm#821)

## Description
It adds handling for the updated data struct `ConsensusParameters` (new
field: `block_transaction_size_limit`). No additional logic around this
new parameter is implemented in this PR, it's just a stub for further
work.

The new parameter is exposed via GraphQL, example:

Request snippet:
```graphql
  consensusParameters(version: 0) {
    blockGasLimit
    blockTransactionSizeLimit
  }
```

Response snippet:
```json
  "consensusParameters": {
    "blockGasLimit": "30000000",
    "blockTransactionSizeLimit": "129024"
  }
```

## Checklist
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [ ] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?

---------

Co-authored-by: green <[email protected]>
Co-authored-by: Hannes Karppila <[email protected]>
xgreenx added a commit that referenced this issue Sep 17, 2024
@xgreenx @rafal-ch @Dentosal
Applying several breaking changes to the WASM interface from backlog (#…
0d084fd 
…2199)

Closes #1887
Closes FuelLabs/fuel-vm#797
Related #2114
Related #2133

This PR contains several small breaking changes to the WASM interface:
- Get the module to execute WASM byte code from the storage first, an
fallback to the built-in version in the case of the
`FUEL_ALWAYS_USE_WASM`.
- Added `host_v1` with a new `peek_next_txs_size` method, that accepts
`tx_number_limit` and `size_limit`.
- Added new variant of the return type to pass the validation result. It
removes block serialization and deserialization and should improve
performance.
- Added a V1 execution result type that uses `JSONError` instead of
postcard serialized error. It adds flexibility of how variants of the
error can be managed. More information about it in
FuelLabs/fuel-vm#797. The change also moves
`TooManyOutputs` error to the top. It shows that `JSONError` works as
expected.
- Use a WASM generated bytecode for the local testnet since we always
want to use the latest version compatible with native execution.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests

### Before requesting review
- [x] I have reviewed the code myself
- [x] I have created follow-up issues caused by this PR and linked them
here
    - #2198
    - #2200

---------

Co-authored-by: Rafał Chabowski <[email protected]>
Co-authored-by: Hannes Karppila <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rafal-ch rafal-ch

Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enforce the block size limit FuelLabs/fuel-core
2 participants
@xgreenx @rafal-ch