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

Document and and draft refactoring proposal for x/bank blocked addresses #8996

Closed
1 of 7 tasks
robert-zaremba opened this issue Mar 25, 2021 · 7 comments
Closed
1 of 7 tasks

Comments

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented Mar 25, 2021

Problem Definition

App defines a list of blocked addresses, which are prohibited to receive transfers.
This features is not documented, and recent issues showed that it's badly designed.

Proposal

  • create a specification for blocked addresses
  • propose a new design
  • implement the new design

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@robert-zaremba
Copy link
Collaborator Author

@aaronc made a suggestion to move a logic for ability to receive coin transfers to Accounts - which is an interesting idea to explore.

@alexanderbez
Copy link
Contributor

alexanderbez commented Mar 25, 2021

Poorly documented, perhaps. But why is it badly designed?

Recall, all accounts can received coins. However, what we're trying to prohibit or control is the ability of certain accounts from directly and explicitly receiving coins. With a direct account-based approach, how do you tackle this? The receiving account needs to know the "intent" of the funds, e.g. are the funds from some inner workings of the state-machine or directly from a user.

@robert-zaremba
Copy link
Collaborator Author

Poorly documented, perhaps. But why is it badly designed?

There is no mention in documentation nor in docstring about blacklisted nor blocked accounts. So we should spec it, describe motivation and describe to which accounts transfers should be blocked.

But why is it badly designed

Because it causes bugs and can't be configured dynamically (seams it's hardcoded) - not sure if this is needed though. Once we will have the spec we can better discuss it.

@aaronc
Copy link
Member

aaronc commented Mar 25, 2021

The way that I am thinking that this is ideally solved is that when an account is created it can block itself in InitGenesis. For instance, if I am the distribution module, I know that I have a module account that cannot receive coins. So in InitGenesis I block my own account from receiving funds and basically specify that only the module can fund its own account. With the ADR 033 approach, I think the way to do this might become clearer.

The basic issue with the way things are currently setup is that it is left up to app configuration to make sure the correct accounts are blocked. But distribution and other modules know they should be blocked from receiving funds. So this should not live at the app configuration level, instead modules should have a way of setting up the appropriate permissions on their accounts so app developers don't need to think about it.

@alexanderbez
Copy link
Contributor

alexanderbez commented Mar 25, 2021

I don't see how that proposal works @aaronc. You need to block "intent", not the actual coins. e.g. mint can receive funds to/from distribution, but neither can receive funds from you or me directly. Correct me if I'm wrong.

@aaronc
Copy link
Member

aaronc commented Mar 25, 2021

@alexanderbez the general principle I'm holding in mind is that those modules know what their rules are and should be able to define those during initialization. App developers should not need to configure something special to make sure the correct rules are in place, which is the way this works with blocked addresses.

@aaronc aaronc changed the title Document and and draft refacore proposal for transfer blocked addresses Document and and draft refactoring proposal for x/bank blocked addresses Mar 25, 2021
@alexanderbez
Copy link
Contributor

Looking forward to seeing the proposal/ADR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants