Cosmos-sdk: move Un/DelegateCoins to x/staking

Created on 5 Apr 2019  Â·  5Comments  Â·  Source: cosmos/cosmos-sdk

Summary

Discuss whether DelegateCoins and UndelegateCoins should live within the bank or staking module.

Problem Definition

Currently the bank keeper handles the delegation and delegation of coins. The thing with this is that as an SDK dev I don't want to use the staking module (say by implementing an own PoA or PoW module). Then this 2 functions are useless:

https://github.com/cosmos/cosmos-sdk/blob/055d2193017953cdf2a015f1f81045b8781de0e4/x/bank/keeper.go#L14-L26

cc: @rigelrozanski @cwgoes @alessio @jackzampolin


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned
proposal-accepted stale staking

Most helpful comment

Yeah I think it could make sense to include these accounting features within the staking module - the ultimate home of delegation logic is in fact staking. Ideally we could somehow abstract out accounting patterns within bank so that any duplicate accounting patterns required for use within staking for the delegation logic can simply come directly from bank.

This said, I think that the fact that the initial build of the delegation logic has been added to bank is totally acceptable as the MVP. This issue simply describes the next evolution

All 5 comments

The staking keeper when then need access to the auth's account keeper...just for this sole purpose. The staking keeper already needed the bank keeper, hence we decided to implement on such. Otherwise, the staking keeper would need to get the account, do the arithmetic, and set the coins. Sounds like a banking operation to me.

@alexanderbez I don't think @fedekunze meant to call into question the design rationale, which is very clear indeed. However in his use case of develping a PoA dapp, as an SDK end-user he wants to use bank features and at the same time avoids exposing staking delegation features.

(I'm CC'ing @sunnya97 @mossid @sabau too)

Sure, so just don't simply use those features 😉. That or update the staking module to do accounting.

Yeah I think it could make sense to include these accounting features within the staking module - the ultimate home of delegation logic is in fact staking. Ideally we could somehow abstract out accounting patterns within bank so that any duplicate accounting patterns required for use within staking for the delegation logic can simply come directly from bank.

This said, I think that the fact that the initial build of the delegation logic has been added to bank is totally acceptable as the MVP. This issue simply describes the next evolution

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hendrikhofstadt picture hendrikhofstadt  Â·  3Comments

jackzampolin picture jackzampolin  Â·  3Comments

mossid picture mossid  Â·  3Comments

ValarDragon picture ValarDragon  Â·  3Comments

ValarDragon picture ValarDragon  Â·  3Comments