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

Token allow list feature. Bank module changes #538

Merged
merged 13 commits into from
Sep 20, 2024
Merged

Conversation

dssei
Copy link
Contributor

@dssei dssei commented Sep 4, 2024

Describe your changes and provide context

This change is part of token allow list feature implementation for token extensions.
Added functions to:

  • persist denom allow list and query it
  • check if from and to addresses are in allowlist on bank send and multi-send

Testing performed to validate your change

  • unit testing
  • functional testing (plugged modified code into sei-chain) and via ERC-20 pointer contract

- persist allow list
- check allow list on send
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 86.81319% with 12 lines in your changes missing coverage. Please review.

Project coverage is 54.88%. Comparing base (6b75f30) to head (f3d02f2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
x/bank/keeper/send.go 89.77% 6 Missing and 3 partials ⚠️
x/bank/types/key.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #538      +/-   ##
==========================================
+ Coverage   54.83%   54.88%   +0.05%     
==========================================
  Files         631      631              
  Lines       54808    54899      +91     
==========================================
+ Hits        30052    30132      +80     
- Misses      22604    22612       +8     
- Partials     2152     2155       +3     
Flag Coverage Δ
54.88% <86.81%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
x/bank/types/key.go 32.50% <0.00%> (-2.64%) ⬇️
x/bank/keeper/send.go 86.02% <89.77%> (+1.18%) ⬆️

... and 1 file with indirect coverage changes

@dssei dssei requested a review from codchen September 6, 2024 04:43
@dssei dssei marked this pull request as ready for review September 6, 2024 04:43
if err != nil {
return err
}
allowedToReceive := k.IsAllowedToSendCoins(ctx, outAddress, out.Coins, denomToAllowListCache)
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it intended to use the same IsAllowedToSendCoins for receiver validation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was going on naming back and forth. But the same method is used for both sender and receiver validation.

Copy link
Collaborator

@codchen codchen left a comment

Choose a reason for hiding this comment

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

has one nit but overall lgtm

…tion to support minting for tokenfactory tokens as tokens are sent from the module address
@dssei dssei merged commit 5081dc1 into main Sep 20, 2024
15 checks passed
@dssei dssei deleted the feature/token_allow_list branch September 20, 2024 17:17
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.

2 participants