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

Generalize auth/type.StdFee #4509

Closed
wants to merge 2 commits into from
Closed

Generalize auth/type.StdFee #4509

wants to merge 2 commits into from

Conversation

alessio
Copy link
Contributor

@alessio alessio commented Jun 7, 2019

A new Fee interface is made available in the top level types package.
auth.StdFee implements such interface. User defined auth module can
now define their own custom fee types.

Work carried out in the context of the following issues:


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

A new Fee interface is made available in the top level types package.
auth.StdFee implements such interface. User defined auth module can
now define their own custom fee types.

Work carried out in the context of the following issues:
- #4488
- #4487
@alessio alessio requested review from sabau and fedekunze June 7, 2019 17:25
Copy link
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather see abstractions of the fee logic to auth and have a generic FeeProcessor interface in types which auth could fulfill. This way the stdFee and all associated logic would live in auth and be readily swap-able with other auth logic sets (edited)
All sort of blockchains are going to want to use radically different fee-mechanisms

@codecov
Copy link

codecov bot commented Jun 7, 2019

Codecov Report

Merging #4509 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4509      +/-   ##
==========================================
+ Coverage   53.23%   53.24%   +0.01%     
==========================================
  Files         259      259              
  Lines       16203    16205       +2     
==========================================
+ Hits         8625     8629       +4     
+ Misses       6932     6930       -2     
  Partials      646      646

@alexanderbez
Copy link
Contributor

alexanderbez commented Jun 7, 2019

Ref: #4512

Second @rigelrozanski here. It seems like we want Authenticator and FeeProcessor interfaces defined in types/ which x/auth/types fulfills.

@alexanderbez
Copy link
Contributor

@alessio bump or close por favor.

@alessio alessio closed this Jun 20, 2019
@alessio alessio deleted the alessio/decouple-StdFee branch June 20, 2019 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants