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

WalletRegistry.seize function implemented #2942

Merged
merged 7 commits into from
Apr 26, 2022
Merged

WalletRegistry.seize function implemented #2942

merged 7 commits into from
Apr 26, 2022

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Apr 22, 2022

Closes #2927

Allows the wallet owner to add all signing group members of the wallet with the given ID to the slashing queue of the staking contract. The notifier will receive reward per each group member from the staking contract notifiers treasury. The reward is scaled by the rewardMultiplier provided as a parameter.

This function will be used by tBTC Bridge contract to slash wallet signing group members.

The gas cost of calling this function when slashing all 100 members of the group is ~3M gas.

This cost was captured after upgrading @threshold-network/solidity-contracts dependency to include the gas cost fix from threshold-network/solidity-contracts#87.

After upgrading the dependency, I had to fix the unit tests for WalletRegistry authorizations to take into account the change of the behavior in the staking contract. This was done in the commit a5a83e7.

Allows the wallet owner to add all signing group members of the wallet with the
given ID to the slashing queue of the staking contract. The notifier will
receive reward per each group member from the staking contract notifiers
treasury. The reward is scaled by the `rewardMultiplier` provided as a parameter.

This function will be used by tBTC Bridge contract to slash wallet signing
group members.
This version contains fixes for the gas cost of slashing.
…g contract

The change in the staking contract modified how authorized stake is reduced
after slashing, making the reduction "global", to all authorized applications,
and not only "local" to a single application, as it was initially.

The original approach was to decrease authorization on the slashing application
and decrease authorizations on other applications only if necessary, that is, if
the staked amount after the slashing was lower than the authorization on the
other application.

For example, let's assume the staking provider has 1000 T delegated and three
applications authorized:
- A application authorized for 950 T
- B application authorized for 500 T
- C application authorized for 100 T

If C slashed for 50 T, authorization on C would be reduced by 50 T,
to 100 - 50 = 50 T.

If A slashed for 950 T, authorizations on A would be reduced to 0, and
authorizations on B and C would be reduced to 50 T.

The new approach assumes all applications are decreasing authorizations in case
of slashing, no matter which application executed the slash.

If C slashed for 50 T, authorization on A would be reduced to 950 - 50 = 900 T,
authorization on B would be reduced to 500 - 50 = 450 T, and authorization on C
would be reduced to 100 - 50 = 50 T.

If A slashed for 950 T, authorizations on A, B, and C would be reduced to 0 T.

The unit tests had to be adjusted to reflect the new reality.

See threshold-network/solidity-contracts#87
The whole point of this function is to be called by wallet owner who is
working with IWalletRegistry interface.
`WalletRegistry.seize` reverts if the staking contract call reverts. The
calling code needs to take it into account and handle the potential
revert properly.
@pdyraga
Copy link
Member Author

pdyraga commented Apr 26, 2022

This PR is ready for another chance @nkuba.

There is no easy way to implement this test right now.
@nkuba nkuba enabled auto-merge April 26, 2022 15:17
@nkuba nkuba merged commit 9c8499c into main Apr 26, 2022
@nkuba nkuba deleted the ecdsa-slashing branch April 26, 2022 15:25
@pdyraga pdyraga removed their assignment Apr 28, 2022
@pdyraga pdyraga added this to the solidity/v2.0.0 milestone Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slashing capabilities for WalletRegistry wallet owner
2 participants