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 more graceful handling of the huge number of transactions during production #2114

Closed
xgreenx opened this issue Aug 20, 2024 · 5 comments
Closed

Add more graceful handling of the huge number of transactions during production #2114

xgreenx opened this issue Aug 20, 2024 · 5 comments
Assignees
@acerone85
Labels
good first issue Good for newcomers

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Aug 20, 2024

The total number of allowed transactions per block is 65535 due to the TxPointer limitations.

Our current network configuration handles it.

But since we plan to have many custom rollups that use fuel-core. It would be nice to add more graceful handling of a huge number of transitions.

We have several places where we need to handle it:
@xgreenx xgreenx added the good first issue Good for newcomers label Aug 20, 2024
@xgreenx xgreenx mentioned this issue Aug 27, 2024
Closed
@acerone85 acerone85 self-assigned this Sep 6, 2024
@acerone85
Copy link
Contributor

I'll start working on this one, will probably add more comments with questions to get context

@acerone85
Copy link
Contributor

What is the expected way this should be handled? Should we have a consensus parameter that mentions the maximum number of transactions per block (in which case this is related to what @rafal-ch is doing with #2133)? Or is there any other approach we can take?

@acerone85
Copy link
Contributor

What is a FTI transaction?

@xgreenx
Copy link
Collaborator Author

xgreenx commented Sep 9, 2024

FTI - Force transaction inclusion. You can submit the transaction to L1 instead of L2(maybe someone censorships your transaction), and it will be included when the block producer increases the DA block height. FTI - is a regular transaction that just comes from L1, instead of TxPool.

This was referenced Sep 9, 2024
Merged
Merged
@xgreenx xgreenx mentioned this issue Sep 15, 2024
Merged
4 tasks
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 <88321181+rafal-ch@users.noreply.github.com>
Co-authored-by: Hannes Karppila <2204863+Dentosal@users.noreply.github.com>
acerone85 added a commit that referenced this issue Sep 19, 2024
@acerone85
TransactionSource: specify maximum number of transactions to be fetch…
79ca0d0 
…ed (#2182)

## Linked Issues/PRs
<!-- List of related issues/PRs -->

Part of #2114 

## Description

- Add a new argument `transactions_limit: u16` to the function
`TxSource::next()`.
- Changes the logic of the block production to never exceed `u16::MAX`
transactions (included FTI transactions and mint transactions) when
selecting L2 transacitons. The number of transactions can still exceed
`u16::MAX` when selecting transactions from L1. This is addressed in
#2189.
- Changes the implementation of all TxSource adapters to fetch a number
of transactions below or equal to the `transaction_limit`.
- Introduces a new WASM host function to peek the next transactions size
for a number of transactions below a specified limit.

Breaks forward compatibility of the genesis transition function. See
https://github.com/FuelLabs/fuel-core/blob/8eeae5d6730d34ae06390b7797478bb98fd07987/version-compatibility/forkless-upgrade/README.md

## TODO:
- [x] Check that requirements are met
- [x] Implementation for the WasmTxSource is missing (only the argument
has been added)
- [ ] Integration Tests

## Checklist
- [ x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] 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
- [x] 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?
acerone85 added a commit that referenced this issue Sep 20, 2024
@acerone85 @xgreenx
Block producer selects da height to never exceed u64::MAX - 1 transac…
ca6a705 
…tions from L1 (#2189)

## Linked Issues/PRs
<!-- List of related issues/PRs -->

Part of #2114

## Description
When producing a block, we want to make sure that we never include more
than 65_536 transactions.
To this end, we select the DA height for including Forced transactions
so that no more than 65_535 transactions will ever be included.

Changes: 

- A new function for the RelayerPort to get the number of forced
transactions at a given level
- Refactor the function `select_new_da_height` to return early if more
than 65_535 transactions will be included.
- Test the changes to the `select_new_da_height` function. 
- 
## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] 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
- [x] 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 Baneling <XgreenX9999@gmail.com>
MitchTurner pushed a commit that referenced this issue Sep 24, 2024
@acerone85 @MitchTurner
TransactionSource: specify maximum number of transactions to be fetch…
b72959c 
…ed (#2182)

## Linked Issues/PRs
<!-- List of related issues/PRs -->

Part of #2114 

## Description

- Add a new argument `transactions_limit: u16` to the function
`TxSource::next()`.
- Changes the logic of the block production to never exceed `u16::MAX`
transactions (included FTI transactions and mint transactions) when
selecting L2 transacitons. The number of transactions can still exceed
`u16::MAX` when selecting transactions from L1. This is addressed in
#2189.
- Changes the implementation of all TxSource adapters to fetch a number
of transactions below or equal to the `transaction_limit`.
- Introduces a new WASM host function to peek the next transactions size
for a number of transactions below a specified limit.

Breaks forward compatibility of the genesis transition function. See
https://github.com/FuelLabs/fuel-core/blob/8eeae5d6730d34ae06390b7797478bb98fd07987/version-compatibility/forkless-upgrade/README.md

## TODO:
- [x] Check that requirements are met
- [x] Implementation for the WasmTxSource is missing (only the argument
has been added)
- [ ] Integration Tests

## Checklist
- [ x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] 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
- [x] 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?
MitchTurner pushed a commit that referenced this issue Sep 24, 2024
@acerone85 @xgreenx @MitchTurner
Block producer selects da height to never exceed u64::MAX - 1 transac…
2aae70e 
…tions from L1 (#2189)

## Linked Issues/PRs
<!-- List of related issues/PRs -->

Part of #2114

## Description
When producing a block, we want to make sure that we never include more
than 65_536 transactions.
To this end, we select the DA height for including Forced transactions
so that no more than 65_535 transactions will ever be included.

Changes: 

- A new function for the RelayerPort to get the number of forced
transactions at a given level
- Refactor the function `select_new_da_height` to return early if more
than 65_535 transactions will be included.
- Test the changes to the `select_new_da_height` function. 
- 
## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] 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
- [x] 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 Baneling <XgreenX9999@gmail.com>
@acerone85 acerone85 reopened this Sep 26, 2024
@acerone85
Copy link
Contributor

acerone85 commented Sep 26, 2024 •
edited
Loading

Reopening, Still need to update the minimal gas price

@acerone85 acerone85 mentioned this issue Sep 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@acerone85 acerone85

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

No branches or pull requests
3 participants
@MitchTurner @xgreenx @acerone85