go/staking: add minimum balance

[pro-wh]

writing down some basic thoughts on the idea of a minimum balance (general balance only)

cross reference https://oasisprotocol.clickup.com/t/1zfn2hb

still a lot of problems with this

[pro-wh]

moving some more out-there ideas to #4466.

changes to this PR:

• authenticate check now checks after moving the fee in memory. if they don't have enough to meet the min balance after moving the fee, even the fee payment gets rolled back
• general post-execute (was written wrong anyway) check removed
• manually add some checks to staking app methods that are known to affect general balance: transfer/withdraw (from + to), add escrow, burn
• rebased on master, no longer including go/staking: add minimum transfer amount  #4456

thus:

• various other non-method places can still move funds to create less-than-minimum balances:
  • fee payment
    • you would have had to send several registry transactions already to become a validator, so maybe this is fine
  • debonding completion
    • you would have had to send a reclaim escrow transaction already, so maybe this is fine
  • anything else that I'm not aware of
    • 😬 reviewers pls enlighten me
• future work better be extra careful
• you can no longer transfer less than the min balance to a new account
  • maybe this obviates go/staking: add minimum transfer amount  #4456 now
• you can't withdraw without already having the min balance + fee

[pro-wh]

gonna try CI 🙌

[pro-wh]

hm ci is hanging on whatever comes after TestCrashingBackendDoNotInterfere

gonna see if I can reproduce that locally

🤦 I can

ah. an obvious impact to huge things: pretty much all nodes are dead due to not having any balance when trying to process a registry transaction

[pro-wh]

fixed a dos risk: there's now a single check for balance >= fee + min balance, shared between check tx and deliver tx. otherwise, an attacker may submit a high-gas-price transaction with fee <= balance < fee + min balance, which would get scheduled with high priority but always abort

[pro-wh]

adding a way to mark some methods as exempt from the min balance check. the suggested use is for transactions routinely used by nodes, which (i) commonly have no balance and (ii) are known in the registry system

[pro-wh]

we discussed more yesterday, and we'd like to extract the configurable minimum balance check, without the method exemptions

[codecov]

Codecov Report

Merging #4461 (893feb3) into master (8b6bafb) will increase coverage by 0.01%.
The diff coverage is 92.20%.

@@            Coverage Diff             @@
##           master    #4461      +/-   ##
==========================================
+ Coverage   68.03%   68.04%   +0.01%     
==========================================
  Files         421      421              
  Lines       46914    46985      +71     
==========================================
+ Hits        31918    31972      +54     
- Misses      11050    11067      +17     
  Partials     3946     3946              

Impacted Files	Coverage Δ
go/consensus/tendermint/beacon/beacon.go	72.24% <0.00%> (ø)
go/staking/api/api.go	62.33% <ø> (ø)
go/consensus/tendermint/apps/staking/state/gas.go	84.05% <80.00%> (-0.16%)	⬇️
...o/consensus/tendermint/apps/staking/state/state.go	64.20% <88.23%> (+0.58%)	⬆️
.../consensus/tendermint/apps/staking/transactions.go	74.28% <100.00%> (+2.61%)	⬆️
go/staking/tests/state.go	95.39% <100.00%> (ø)
go/oasis-node/cmd/common/metrics/disk.go	65.51% <0.00%> (-20.69%)	⬇️
go/worker/common/committee/runtime_host.go	58.33% <0.00%> (-8.34%)	⬇️
go/oasis-node/cmd/common/metrics/resource.go	84.00% <0.00%> (-8.00%)	⬇️
go/runtime/host/protocol/connection.go	64.66% <0.00%> (-6.77%)	⬇️
... and 23 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2139fb4...893feb3. Read the comment docs.

[pro-wh]

in the process of adding a big e2e test

right now it passes, I think because the nodes are registered in the genesis state. if the test would run longer, they'd all fail to re-register

[pro-wh]

tests added

[kostko]

You should probably also add checks to the staking state's Transfer method (used e.g. by roothash.SubmitMsg transaction method).

[pro-wh]

oh thanks. I didn't know there was a separate code path there

edit: or rather not, this seems to be a different level of abstraction