Skip to content

Introduce stake jobs batching and delayed execution#1525

Merged
sam0x17 merged 34 commits intodevnet-readyfrom
aggregate-stakes
Apr 23, 2025
Merged

Introduce stake jobs batching and delayed execution#1525
sam0x17 merged 34 commits intodevnet-readyfrom
aggregate-stakes

Conversation

@shamil-gadelshin
Copy link
Collaborator

@shamil-gadelshin shamil-gadelshin commented Apr 10, 2025

Description

This PR introduces changes to stake-related extrinsics (starting with add_stake). The goal is to address MeV behavior by aggregating stake operations into batches according to their type before executing, thus removing the possibility of the "sandwich" MeV.

The PR introduces a new storage map (StakeJobs) which accumulates the stake operations (jobs) during the extrinsic calling. Those jobs will be executed during the on_finalize() call in the end of the block.

Related Issue(s)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please describe):

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have run cargo fmt and cargo clippy to ensure my code is formatted and linted correctly
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional Notes

The extrinsic contains parts of the code behind the runtime-benchmark feature to enable the correct weight calculation considering the on_finalize() code.

@shamil-gadelshin
Copy link
Collaborator Author

Some comments, as well as the benchmarking code, are missing. I will add those after the initial review.

@shamil-gadelshin
Copy link
Collaborator Author

shamil-gadelshin commented Apr 15, 2025

Recent changes

  • new extrinsics remove_stake_aggregate, remove_stake_aggregate_limit, add_stake_aggregate_limit
  • more tests, including crucial stake job ordering test
  • benchmarks for add_stake_aggregate, remove_stake_aggregate
  • updates for SignedExtension
  • updates for InstanceFilter for ProxyType

Additional notes

  • CallType enum and usage was not modified (also original add_stake_limit and remove_stake_limit weren't present there also)
  • Weights for add_stake_limit_aggregate and remove_stake_limit_aggregate were copied from add_stake_aggregate and remove_stake_aggregate similar to their originals. The new benchmarks for the remaining two extrinsics will be created in a separate PR.

@shamil-gadelshin shamil-gadelshin added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Apr 15, 2025
@shamil-gadelshin shamil-gadelshin marked this pull request as ready for review April 15, 2025 09:56
@shamil-gadelshin shamil-gadelshin marked this pull request as draft April 15, 2025 10:08
@shamil-gadelshin shamil-gadelshin marked this pull request as ready for review April 15, 2025 10:42
Co-authored-by: Loris Moulin <45130584+l0r1s@users.noreply.github.com>
Comment on lines 143 to 151
if cfg!(feature = "runtime-benchmarks") {
Self::stake_into_subnet(
&hotkey,
&coldkey,
netuid,
tao_staked.saturating_to_num::<u64>(),
fee,
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice way to handle it 👍

@shamil-gadelshin
Copy link
Collaborator Author

The last changes introduce aggregated stake events and move "alpha decrease" operation back to extrinsics.

@sam0x17 sam0x17 requested a review from l0r1s April 15, 2025 15:50
l0r1s
l0r1s previously approved these changes Apr 15, 2025
Copy link
Collaborator

@l0r1s l0r1s left a comment

Choose a reason for hiding this comment

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

👍

@camfairchild camfairchild added the breaking-change This PR introduces a noteworthy breaking change label Apr 15, 2025
l0r1s
l0r1s previously approved these changes Apr 23, 2025
Copy link
Collaborator

@l0r1s l0r1s left a comment

Choose a reason for hiding this comment

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

Mostly nits about missing doc, lgtm 👍

Copy link
Collaborator Author

@shamil-gadelshin shamil-gadelshin left a comment

Choose a reason for hiding this comment

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

I copied the original doc comments with that error. Thanks for the review. I changed the original doc comments as well.

@shamil-gadelshin shamil-gadelshin requested a review from l0r1s April 23, 2025 12:05
l0r1s
l0r1s previously approved these changes Apr 23, 2025
gztensor
gztensor previously approved these changes Apr 23, 2025
sam0x17
sam0x17 previously approved these changes Apr 23, 2025
@shamil-gadelshin shamil-gadelshin dismissed stale reviews from sam0x17, gztensor, and l0r1s via bcbf59e April 23, 2025 15:36
@sam0x17 sam0x17 merged commit 1e568eb into devnet-ready Apr 23, 2025
26 checks passed
This was referenced Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This PR introduces a noteworthy breaking change skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants