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 the multi-batching pallet #7

Merged
merged 24 commits into from
Apr 26, 2024
Merged

Add the multi-batching pallet #7

merged 24 commits into from
Apr 26, 2024

Conversation

niksaak
Copy link
Collaborator

@niksaak niksaak commented Mar 5, 2024

This PR adds the Multibatching pallet to support batching calls from multiple callers, approved off-chain, in a single extrinsic. Documentation / tests / benchmarks included. The weights file was generated on a local machine.

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Mar 5, 2024

User @niksaak, please sign the CLA here.

@Moliholy Moliholy force-pushed the feature/pallet-multibatching branch from ae7109a to 584abfd Compare March 18, 2024 10:24
@Moliholy
Copy link
Collaborator

Rebase to latest main branch and fixed formatting errors.

pallets/multibatching/src/lib.rs Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Show resolved Hide resolved
pallets/multibatching/src/lib.rs Show resolved Hide resolved
pallets/multibatching/src/lib.rs Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
Copy link

@gupnik gupnik left a comment

Choose a reason for hiding this comment

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

Overall, makes sense to me. Would be good to add some tests and ensure that benchmarking is setup correctly.

@niksaak niksaak force-pushed the feature/pallet-multibatching branch from a67ae30 to 0cd85cf Compare March 28, 2024 09:51
@niksaak niksaak marked this pull request as ready for review April 16, 2024 19:41
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/mock.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/mock.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
pallets/multibatching/src/lib.rs Outdated Show resolved Hide resolved
@niksaak
Copy link
Collaborator Author

niksaak commented Apr 25, 2024

Note: Got a really weird test failure here https://github.com/paritytech/project-mythical/pull/7/files#diff-f32c6058a961ae704009f331f11355de75895fb1291ab2892e1bc9954495f489R714 in the latest commit. Success where there should be an "Invalid Transaction" error. Doesn't reproduce locally.

@niksaak
Copy link
Collaborator Author

niksaak commented Apr 25, 2024

Tests passed on re-try. Possibly the reason for the previous failure was very lucky key generation in tests.

@Moliholy
Copy link
Collaborator

Please run cargo clippy -p pallet-multibatching --all-features and fix the issues. After that I think we are good to go.

@niksaak niksaak requested a review from Moliholy April 25, 2024 19:49
@niksaak niksaak merged commit 71e5c8e into main Apr 26, 2024
2 of 3 checks passed
@niksaak niksaak deleted the feature/pallet-multibatching branch April 26, 2024 14:17
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.

4 participants