Skip to content

Changing add stake operation - #3196

Open
ii-cruz wants to merge 2 commits into
albatross-v3from
iicruz/fix-staker-dos
Open

Changing add stake operation#3196
ii-cruz wants to merge 2 commits into
albatross-v3from
iicruz/fix-staker-dos

Conversation

@ii-cruz

@ii-cruz ii-cruz commented Dec 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

What's in this pull request?

  • Adds the Minimum stake to the add_stake operation. Add stake transactions have an intrinsic check for their value => min_stake and thus are rejected before they can break the invariant.
  • The idea is that this minimum prevents indefinitely small value add_stake transactions from hindering the retirement.
  • add_stake assumes that the staker wants to have auto-staking as long as there is an active stake. Once no active stake exists, the funds are credited directly to the inactive balance.
  • The idea is to prevent DoS of the redelegation of stake.
  • Adds more unit tests for legacy add stake and the new add stake policy
  • Creates the protocol version gate for the add_stake and intrinsic tx verification

This PR needs #3800 merged first

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@ii-cruz ii-cruz added the bug Something isn't working label Dec 13, 2024
@ii-cruz ii-cruz self-assigned this Dec 13, 2024
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch from e62eccb to 94256d9 Compare January 7, 2025 14:06
paberr
paberr approved these changes Mar 30, 2025
View reviewed changes

@paberr paberr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me

Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
@sisou

sisou commented Mar 30, 2025

Copy link
Copy Markdown
Member

When add_stake adds to the inactive balance (when there's no active balance anymore and the inactive one is the biggest), I assume it doesn't reset the inactiveFrom block number?

@paberr

paberr commented Mar 30, 2025

Copy link
Copy Markdown
Member

It doesn't reset it.

Also, while discussing with @ii-cruz , one thing that is missing is a version gate for the upgrade.
The new behavior should only be active when we have a new version.

@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch from 94256d9 to bfb490c Compare August 1, 2025 18:16
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch 9 times, most recently from 9d0d70d to 91342a2 Compare August 22, 2025 14:23
@ii-cruz
ii-cruz marked this pull request as ready for review August 22, 2025 16:15
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch from 91342a2 to 2573ee9 Compare August 22, 2025 16:16
@ii-cruz ii-cruz added this to the Next upgrade milestone Aug 22, 2025
@ii-cruz
ii-cruz requested a review from paberr August 22, 2025 16:19
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch from 2573ee9 to 8ec152e Compare August 25, 2025 14:19
paberr
paberr reviewed Aug 27, 2025
View reviewed changes

@paberr paberr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally looks good to me

Comment thread primitives/account/tests/staking_contract/add_stake_policy.rs
Comment thread primitives/transaction/src/account/staking_contract/structs.rs
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch 5 times, most recently from 1b0174e to f480b1e Compare September 1, 2025 15:47
@ii-cruz
ii-cruz requested review from paberr and sisou September 1, 2025 15:55
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch 10 times, most recently from 056e6be to 85e391b Compare June 12, 2026 09:52
@ii-cruz
ii-cruz changed the base branch from albatross to iicruz/upgrade-mechanim-tx-verification June 13, 2026 07:32
@ii-cruz
ii-cruz force-pushed the iicruz/upgrade-mechanim-tx-verification branch 8 times, most recently from 629f40d to 7b3521e Compare June 17, 2026 15:31
@ii-cruz
ii-cruz force-pushed the iicruz/fix-staker-dos branch 2 times, most recently from b290e0f to ba1f015 Compare June 17, 2026 17:45
@ii-cruz
ii-cruz force-pushed the iicruz/upgrade-mechanim-tx-verification branch 2 times, most recently from 597b5e2 to ed06e3b Compare June 18, 2026 15:41
@ii-cruz ii-cruz mentioned this pull request Jun 22, 2026
Merged
3 tasks
paberr
paberr requested changes Jul 16, 2026
View reviewed changes

@paberr paberr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some minor comments to be addressed. Overall looks good.

Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
Comment thread primitives/account/tests/staking_contract/staker_add_stake_policy.rs Outdated
Comment thread primitives/account/tests/staking_contract/staker_add_stake_policy.rs Outdated
Comment thread primitives/account/src/account/staking_contract/receipts.rs Outdated
viquezclaudio
viquezclaudio reviewed Jul 23, 2026
View reviewed changes
Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
paberr
paberr reviewed Jul 24, 2026
View reviewed changes

@paberr paberr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some minor comments from my side

Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
Comment thread primitives/account/src/account/staking_contract/receipts.rs Outdated
Comment thread primitives/account/src/account/staking_contract/staker.rs
Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
Comment thread primitives/src/policy.rs Outdated
Comment thread primitives/transaction/src/account/staking_contract/structs.rs Outdated
paberr
paberr requested changes Jul 30, 2026
View reviewed changes

@paberr paberr left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mostly minor things, mostly typos. Except one bigger item in the receipts.

Comment thread primitives/account/src/account/staking_contract/traits.rs Outdated
Comment thread primitives/account/src/account/staking_contract/receipts.rs Outdated
Comment thread primitives/account/src/account/staking_contract/receipts.rs Outdated
Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
Comment thread primitives/account/src/account/staking_contract/staker.rs Outdated
Comment thread primitives/account/src/logs.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@viquezclaudio viquezclaudio viquezclaudio left review comments
Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@paberr paberr paberr requested changes
@sisou sisou Awaiting requested review from sisou

Assignees

@ii-cruz ii-cruz

Labels

bug Something isn't working Protocol Upgrade

Projects

None yet

Milestone

Next upgrade

Development

Successfully merging this pull request may close these issues.

5 participants

@ii-cruz @sisou @paberr @viquezclaudio