Skip to content

feat(cwgrants): Implement x/cwgrants module#527

Merged
fdymylja merged 31 commits into
mainfrom
fd/cw_grants
Jan 16, 2024
Merged

feat(cwgrants): Implement x/cwgrants module#527
fdymylja merged 31 commits into
mainfrom
fd/cw_grants

Conversation

@fdymylja

@fdymylja fdymylja commented Jan 2, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@codecov

codecov Bot commented Jan 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 137 lines in your changes are missing coverage. Please review.

Comparison is base (58575f1) 65.61% compared to head (6c3505b) 64.50%.

Files Patch % Lines
x/cwfees/types/cw.go 0.00% 37 Missing ⚠️
x/cwfees/keeper.go 68.05% 15 Missing and 8 partials ⚠️
x/rewards/ante/fee_deduction.go 32.35% 22 Missing and 1 partial ⚠️
x/cwfees/module.go 46.87% 16 Missing and 1 partial ⚠️
x/cwfees/types/msgs.go 0.00% 16 Missing ⚠️
x/cwfees/msg_server.go 7.69% 12 Missing ⚠️
x/cwfees/query_server.go 12.50% 7 Missing ⚠️
x/cwfees/types/codec.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #527      +/-   ##
==========================================
- Coverage   65.61%   64.50%   -1.12%     
==========================================
  Files          80       88       +8     
  Lines        4633     4851     +218     
==========================================
+ Hits         3040     3129      +89     
- Misses       1436     1556     +120     
- Partials      157      166       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread x/cwgrant/types/cw.go
Comment thread proto/archway/cwgrant/v1/cwgrant.proto
Comment thread x/cwgrant/keeper.go Outdated
Comment thread x/cwgrant/types/keys.go Outdated
Comment thread x/rewards/ante/fee_deduction.go Outdated
Comment thread x/cwgrant/types/cw.go
@fdymylja fdymylja marked this pull request as ready for review January 9, 2024 12:09
@fdymylja fdymylja requested review from a team and spoo-bar January 9, 2024 12:09
# Conflicts:
#	CHANGELOG.md
#	app/app.go
#	app/keepers/keepers.go
#	app/upgrades/latest/upgrades.go
Comment thread contracts/cwfees/artifacts/checksums_intermediate.txt
Comment thread docs/adr/ADR-009-cw-fees.md Outdated
Comment thread docs/adr/ADR-009-cw-fees.md Outdated
Comment thread x/cwfees/types/cw.go
// SudoMsg defines the message sudo enum that is sent to the CosmWasm contract.
type SudoMsg struct {
// CWGrant defines the enum variant of the grant message.
CWGrant *CWGrant `json:"cw_grant"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CWGrant to CWFees?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think CWGrant is fine here, whilst the "module name" is CWFees (as it handles fees); what is being sent as request here is a request for a fee grant.

The name is kept as short as possible to avoid extra gas consumption when we traverse the wasm boundary.

@zanicar zanicar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we going to change CWGrant to CWFees or are we leaving that for the remediation / refactor cycle? LMKWYT

@fdymylja

Copy link
Copy Markdown
Contributor Author

@zanicar I would propose we keep as is, and consider renaming this cwfeegrants, to be in line with cosmos sdk's fee grant module, at nomenclature level, I will create an issue.

@zanicar zanicar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@fdymylja fdymylja merged commit 690d44f into main Jan 16, 2024
@fdymylja fdymylja deleted the fd/cw_grants branch January 16, 2024 13:49
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