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

Proactive protection against GovAction deposit losses #4605

Open
gitmachtl opened this issue Sep 6, 2024 · 9 comments · May be fixed by #4639
Open

Proactive protection against GovAction deposit losses #4605

gitmachtl opened this issue Sep 6, 2024 · 9 comments · May be fixed by #4639
Assignees
Labels
conway intra-era-hardfork A task that requires an intra-era hard fork

Comments

@gitmachtl
Copy link

gitmachtl commented Sep 6, 2024

Current situation:

We have a guard present that disallows you to deregister a stakeaddress if there is still a rewards-balance > 0 in the ledger-state.

We also had a change in the way stakeaddress-registration must be signed, needs the signing with the stake.skey now. Because the stakeaddress deposit fee can change over time, and we wanna make sure that we refund the correct deposit amount to the owner of the address.

So, it comes as a bit of a surprise that there are currently no safety guards when it comes to stakeaddresses used in governance action proposals.

  1. We have the scenario that someone uses a wrong stake address as the deposit return address. There is currently no need to sign a governance transaction also with the stake secret key. Once submitted, there is no way to correct the wrong address anymore afterwards. Deposit funds would be lost.

  2. Another scenario is, that someone is doing a stakeaddress deregistration while that same stakeaddress is involved in an active governance proposal. Once the proposal times out or gets enacted, the deposit fee (currently 100kAda) would be lost.

  3. There is also the scenario with a wrong or deregistered stakeaddress in a treasury withdrawal action. For the treasury payout stakeaddress, that would not be a big issue, the funds would simply go back into the treasury. So we may not have to cover that.

We should introduce some safety mechanism into the ledger, to proactively restrict users from submitting deregistration certificates while there are active governance actions are present involving the same stakeaddress.
To make sure, that noone can "block" a strangers stakeaddress, or that users submit an action with a typo in the stakeaddress, we might wanna think about to add the stakekey signing as a required signature for a action submit transaction too.

There is also a request in the cardano-cli repo to add the currently locked up actionDepositFee to the stake-address-info query (IntersectMBO/cardano-cli#592)

Best regards,
Martin

@lehins
Copy link
Collaborator

lehins commented Sep 6, 2024

I've mentioned this before that all of those guards can be implemented on he tooling side.
However, point #1 is certainly something we should try and protect against, just because deposit for proposals is quite significant. Loosing it for the proposer would be devastating.
So, I suggest we add a predicate failure for the next intra-era hard fork that checks that reward accounts specified for deposit in all of the proposals and in TreasuryWithdrawals are indeed registered.

With respect to preventing deregistration of staking credentials if they are used in other places in the system is not something that we are going to do for two reasons:

  1. It would be an expensive operation
  2. Anyone could prevent staking credential from being deregistered, by using it for their own (proposal, pool or treasury withdrawal). It is highly unlikely that someone would use a proposal for that today 😄, but nevertheless we never know if the deposit will always be that high for proposals.

@gitmachtl
Copy link
Author

gitmachtl commented Sep 6, 2024

Isn't it somehow possible to store the action deposit fee also in the rewards-account-balance? Not directly for a withdrawal, but the amount will be there once the proposal is over. So after a action deposit, this amount will come back to that rewards-account no matter what. In the case of a deregistration attempt, couldn't the node simply lookup in that additional rewards-acount-balance storage and refuse to continue as long as there is a value present? With a flag like we have it for mark, set, etc. ? And that balance is cleared out once the value is moved over to the normal rewards-balance after an action proposal was closed.

Couldn't such an extra value be stored simply in an array like format. So it would only generate more memory usage for the addresses that have active proposals running, and not affecting all rewards-balances? Basically having an array of reward-balances. First entry is the one that the user can withdrawal, the normal rewards-balance we use currently. And all other entries are lined up to become available later on.

But i guess its not that simple right?

@lehins
Copy link
Collaborator

lehins commented Sep 6, 2024

Of course, this is technically doable, but:

  • first of all, it is not as simple as it sounds. We'd have to track not only which reward accounts are being used, but also by which entities: which proposals, stake pools, treasury withdrawals. This is because when any of them are retired we'd have to remove those exact instances from that Map. So, it would have to be a BiMap for efficient lookups.
  • secondly, this whole complexity comes at a computational cost and memory overhead.

We want to avoid making Cardano slower and overly complex, especially when a problem can be solved in tooling.

Most importantly, however, despite that it is technically possible to implement such a guard, there is a fundamental issue with this.
Let's say you have a reward account r1. And it so happens that I really don't like you for some reason, so much so that I am willing to pay money to mess with you. Then I can register a stake pool and specify your account r1 as an account where my deposit should be returned. Until I deregister my pool, which may never happen, you would not be able to deregister you r1 account and get your 2 ADA deposit back.

I know that it doesn't sound very plausible, but in our job we have to look at all possibilities. And this one, despite being not very probable, would be theoretically possible. And we can't allow anyone out there to lock you out of your funds, regardless of how small those funds might be.

My personal opinion is that if someone is dumb enough to go out of their way and submit a transaction that deregisters their reward account, where their 100K ADA deposit should go back to, then they don't deserve getting their deposit back 😁
Joking aside, of course, mistakes happen, and if such deregistration was indeed a mistake, all it takes is for that reward account is to be reregistered before the epoch boundary in which proposal is expired or enacted and they will still get their deposit back.

@lehins
Copy link
Collaborator

lehins commented Sep 6, 2024

We should introduce some safety mechanism into the ledger, to proactively restrict users from submitting deregistration certificates while there are active governance actions are present involving the same stakeaddress.
To make sure, that noone can "block" a strangers stakeaddress, or that users submit an action with a typo in the stakeaddress, we might wanna think about to add the stakekey signing as a required signature for a action submit transaction too.

In other words, none of this is really worth the complexity it entails. It is much simpler and cheaper for a wallet to implement this sort of guard to protect their users from making mistakes.

@lehins lehins added conway intra-era-hardfork A task that requires an intra-era hard fork labels Sep 6, 2024
@lehins lehins changed the title Proactive protection against ActionDeposit losses - Block StakeAddress deregistration Proactive protection against GovAction deposit losses Sep 6, 2024
@gitmachtl
Copy link
Author

gitmachtl commented Sep 7, 2024

so, why can't we than add the stakekey as a required signing to a stakepool registration for the rewads account? we had this in the documentation/tutorials for a long time as a mistake. also making the signing via stakekey mandatory for a transaction containing an action proposal? that would help and also make it highly unlikely that you use the wrong stakeaddress for the deposit return. and/or at least have a check at the time of submit that this address exists on chain.

to be real, if someone add my stakeaddress to there pool as a rewards account, i'll take it. thats free money.

but, submitting a typo by mistake and loosing 100kAda is not a joke. we care about 2 Ada, so we def. should care about such a way higher amount. tooling can improve it by adding those guards for sure, but it should be also done on the ledger level imo.

@Hornan7
Copy link

Hornan7 commented Sep 7, 2024

This nearly happened to us during the very first mainnet governance action. We were about to submit an info action with an unregistered stake address but fortunately remembered at the last minute that we needed to register it first. That was a close call.

We definitely need a mechanism to prevent such mistakes. Specifically, if an address isn’t registered when submitting a governance action, the system should block the transaction from being submited.

Regarding de-registration during the governance action lifetime, would it be possible to keep the 100k ADA in the same pot even after the govActionLifetime expires? The only way to retrieve it would be by signing a specific transaction with the owner's stake.skey. This way, the governance deposit pot could serve as a temporary emergency "reward address" for cases where a stake address is mistakenly unregistered.

Does that make sense, or am I off track?

@lehins
Copy link
Collaborator

lehins commented Sep 7, 2024

would it be possible to keep the 100k ADA in the same pot even after the govActionLifetime expires? The only way to retrieve it would be by signing a specific transaction with the owner's stake.skey.

This is also technically possible, but that is a huge change that requires a whole new era, changes to plutus context, etc. Moreover then the same question arises, why only governance actions? Why not create this protection for the pool deposits?

My answer is again: ledger does not need to solve this problem if it can be solved in the tooling!

Wallets and cli tools that build transactions can easily check if reward account is being used and prevent it from being deregistered.

This feature that is being requested is not needed for safety of Cardano, it is needed to protect users from mistakes and wallets are a great place for this.

The only reason why I am willing to accept deviation from the spec and adding a check for presence of reward account for the proposal deposit, is because it is very easy to do that has almost no overhead on the Ledger.

We were about to submit an info action with an unregistered stake address but fortunately remembered at the last minute that we needed to register it first. That was a close call.

You could have registered the stake address after you submitted the proposal and you would have been fine. As long as the stake address is registered when proposal is expired you don't loose the deposit.

@lehins
Copy link
Collaborator

lehins commented Sep 7, 2024

we care about 2 Ada, so we def. should care about such a way higher amount.

You totally missed my point, but that's ok. It's not my job to convince others.

but it should be also done on the ledger level imo.

I appreciate your opinion. But none of this is necessary for the safety of the system.

When you send ADA to a wrong address that does not exist, you also loose that money. So, it is not our job to prevent people from making typos or other dumb mistakes.

I gave you my expert explanation on why we are not going to do this. I'd appreciate if you stop pushing your opinion. If you don't agree with what I said, feel free to submit a CIP and if there is support form the community on changing the system in the way you are suggesting, it will get implemented. Until then this is a wallets' issue, not ledger's.

@Hornan7
Copy link

Hornan7 commented Sep 7, 2024

You could have registered the stake address after you submitted the proposal and you would have been fine. As long as the stake address is registered when proposal is expired you don't loose the deposit.

Oh! Awesome I wasn't sure about that TBH. Thank you. 🤩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conway intra-era-hardfork A task that requires an intra-era hard fork
Projects
Status: In review
4 participants