Cosmos-sdk: Interface based implementation for all packages

[jay-dee7]

Summary

Hey guys, I was trying to play with x/distribution package and i came across this implementation of Keeper with struct and not interface which i think is not encouraged as it doesn't allows extendibility. Can we please have interface based implementation for the packages so that if we people want to extend some functionality specific to their own chain/app, it would be much easier.

Problem Definition

type Keeper struct {
  storeKey            sdk.StoreKey
  cdc                 *codec.Codec
  paramSpace          params.Subspace
  bankKeeper          types.BankKeeper
 ...
}


#### returning a struct type 

func NewKeeper(....) Keeper {
  keeper := Keeper{
    storeKey:            key,
    cdc:                 cdc,
    paramSpace:          paramSpace.WithKeyTable(ParamKeyTable()),
    bankKeeper:          ck,
    stakingKeeper:       sk,
    feeCollectionKeeper: fck,
    codespace:           codespace,
  }

  return keeper
}

func (k Keeper) SetWithdrawAddr(...) sdk.Error {
    /*
      Method implementation
    */
}

Proposal

type Keeper interface {
  // method documentation
  SetWithdrawAddr(...) sdk.Error
 ...
}

type distKeeper struct {
    storeKey            sdk.StoreKey
  cdc                 *codec.Codec
  paramSpace          params.Subspace
  bankKeeper          types.BankKeeper
....
}

## returning Interface type instead of struct

func NewKeeper(....) Keeper {
  return &Keeper{
    storeKey:            key,
    cdc:                 cdc,
    paramSpace:          paramSpace.WithKeyTable(ParamKeyTable()),
    bankKeeper:          ck,
    stakingKeeper:       sk,
    feeCollectionKeeper: fck,
    codespace:           codespace,
  }
}

func (dk distKeeper) SetWithdrawAddr(...) sdk.Error {
    /*
        Method implementation
    */

}

For Admin Use

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

[alexanderbez]

@jay-dee7, module keepers are implemented internally x/{module}/internal/keeper. When another module requires access or usage of a specific keeper, it's passed by _interface_ already through the expected_keepers.go pattern (take a look at some of the modules -- we already do this).

With this in mind, what is the motivation then for having module keepers implemented as interfaces _internally_? Not saying I'm against it, but I would like to see some thought experiments on the benefits it brings.

For example, in x/distr if you turned that keeper into an interface. How would you extend it and for what purpose?

/cc @fedekunze @rigelrozanski

[jay-dee7]

@alexanderbez i suggested this for better standardization as even Go's standard lib follows this pattern. However assume as, along with normal distribution of atoms for running validators, you want to give an option for your validators to stake some percentage of the reward for a certain period time and boost their ranking/credibility or even gain some sort of interest too if they stake. Basically people can have their own implementation of keepers for whatever use case they might have, all they have to do is satisfy the interface. Plug n play.

[alexanderbez]

i suggested this for better standardization as even Go's standard lib follows this pattern.

What exactly are you suggesting and what pattern do you mean in Go's standard lib? Go's stdlib has many packages, types, and interfaces. Which exact pattern?

Basically people can have their own implementation of keepers for whatever use case they might have, all they have to do is satisfy the interface. Plug n play.

Again, we already allow this to some extent by passing interfaces to keepers when they need access to other keepers. In fact, no module right now depends on distribution apart from staking's hooks (which is already an interface).

The existing distribution module in the SDK implements a specific distribution protocol -- F1. Afaik, it's not meant to plug-n-play really. How would implementing the distribution keeper as an interface help in this regard? Perhaps you can provide the interface for which you'd like to satisfy so I can get a better understanding?

[rigelrozanski]

Yeah I don't think internal keeper interfaces make much sense.

Let's say I wanted to extend the distribution keeper this is how I would do it:

• make my own really-cool distribution module sale cooldistr
• have cooldistr's keeper extend the distribution keeper
• overwrite any functions I want to be different:

type CoolKeeper struct {
    ExpectedDistributionKeeper
}

func NewCoolKeeper (oldDistrKeeper ExpectedDistributionKeeper) Keeper {
    return CoolKeeper{oldDistrKeeper}
}

func (ck CoolKeeper) SetWithdrawAddr(ctx sdk.Context, delegatorAddr sdk.AccAddress, 
    withdrawAddr sdk.AccAddress) sdk.Error {

    // by including this function I'm overwriting 
    // the original functionality of the ExpectedDistributionKeeper
}

[fedekunze]

@rigelrozanski totally agree with your approach. I think we need to add documentation or update the tutorial to explain how to extend existing keepers and handlers. cc: @marbar3778 @okwme @jackzampolin @hschoenburg

[fedekunze]

closing as there are no more actionable items here