Allow app devs to create Fee Allowances with access to custom data

[amaury1093]

Summary

Allow app devs to create Fee Allowances with access to custom data

Problem Definition

Right now, all possible fee allowances are hardcoded in the SDK: Basic, Periodic, AllowedMsg. We should allow app developers to create their own, i.e. any proto.Message struct that implements FeeAllowanceI interface.

The challenge comes from the keeper.UseGrantedFees function, which currently has the following signature:

func (k Keeper) UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msgs) error {}

It's impossible for an app developer to come up with a FeeAllowances that needs some data that is not in the function arguments.

Proposal

Maybe the last argument could be an empty interface?

func (k Keeper) UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, customFA interface{}) error {}

and also same thing for FeeAllowanceI's Accept:

interface FeeAllowanceI {
  Accept(fee sdk.Coins, blockTime time.Time, blockHeight int64, customFA interface{}) (remove bool, err error)
}

As an example, let's see how the AllowedMsgsFeeAllowance (#8604) could be implemented using this proposal:

The antehandler passes a custom fee allowance into the UseGrantedFees last argument:

// my/custom/feegrant/antehandler.go

customFA := types.AllowedMsgsFeeAllowance{...}
decorator.feeKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, customFA)

Then, in the Accept() method of AllowedMsgsFeeAllowance, we would have:

func (a *AllowedMsgFeeAllowance) Accept(fee sdk.Coins, blockTime time.Time, blockHeight int64, customFA) (bool, error) {
  allowedMsgsFA := customFA.(*types.AllowedMsgsFeeAllowance)
  // business logic as usual
}

And of course, customFA could be any FeeAllowance (or set of FeeAllowances) the app developer can think of.

Notes

• initial idea from @robert-zaremba, feel free to add more info
• should customFA interface{} be a simple interface instead of an empty one?

---

For Admin Use

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

[robert-zaremba]

Thanks @AmauryM for drafting this.
I would strongly prefer to not use empty interface. Instead, maybe we can have some builder? I will try to find more time to look at the code.

[aaronc]

I'm not sure why you're saying custom fee allowances aren't possible. You can create another message that implements FeeAllowanceI and it should work right? It just wouldn't have access to other data. What's the issue?

[amaury1093]

It just wouldn't have access to other data

That's the issue, I renamed the title.

[aaronc]

So the idea is that some app developer wants to pass something custom thing down to fee allowances? That gets type cast into a few allowance...?

I have to say I'm pretty confused by this proposal. What is the use case for this?

[amaury1093]

An example use case (that I just made up), LowBalanceFeeAllowance: "granter allows grantee a fee allowance as long as grantee's balance is under X tokens". Today, it's not possible to create such a fee allowance.

With the above proposal, the customFA interface in this case would then be a struct{bankKeeper}, so that the LowBalanceFeeAllowance's Accept method has access to the bankKeeper.

---

The above use case is made up. This issue is about allowing any imaginable FeeAllowanceI be able to access any data.

[aaronc]

I would like to have some clear concrete use cases outlined before adding features like this. And second I dislike having just a interface{} that gets cast, it seems like it adds a ripe surface area for bugs. Also, the approach of allowing ADR 033 queries in Accept I think would address what you're thinking of but with more type safety, no?

[robert-zaremba]

Let me reopen. The ADR-33 approach would work if an FeeAllowanceI.Accept would be able to get a proto client from the context. Are we planning to add all clients to the context?