Add ability to quarantine and sanctioned accounts.

[SpicyLemon]

Summary

Add the ability for account owners to opt into quarantine, preventing any funds from coming into their account until they have approved the transfer.

Add the ability to manage a list of sanctioned accounts. All funds in a sanctioned account are locked, and cannot be transferred.

Problem Definition

Quarantine: As an account owner, I want to prevent funds from being transferred to my account without my approval, so that I don’t end up with funds that I don’t want and which might even be problematic.

Sanction: As a regulated entity trying to do business using a blockchain, I want to be able to lock an account, preventing funds from being sent from it, so that the chain can comply with government sanctions and I can continue to use the blockchain within the law.

Proposal

1. Create a x/quarantine module to facilitate quarantine-related stuff.
2. Create a x/sanction module to facilitate sanction-related stuff.
3. Update the x/bank module to enforce quarantines and sanctions.

A more thorough writeup can be found here: https://hackmd.io/@dwedul/BJuR4aQDj

[SpicyLemon]

I've done most of the work for the quarantine module on the Provenance fork. The draft PR can be found here: provenance-io#335 As of writing this, I just need unit tests on the new bank stuff, simulation stuff in the quarantine module and spec documentation. All the rest of the functionality is there though.

[alexanderbez]

Interesting ideas @SpicyLemon! But why do these need to exist in the SDK? Can't applications import Provenance's modules?

[SpicyLemon]

Technically, the quarantine and sanction modules don't need to live in the SDK. However, they need to be enforced by the bank module.

There's three spots in the bank module that would need restrictions applied:

1. In SendCoins for quarantine.
2. In InputOutputCoins for quarantine.
3. In subUnlockedCoins for sanction.

I've been trying to come up with a generic way to inject restrictions into the bank module's stuff, but I've come up empty. The sanction restriction could technically be done in both SendCoins and InputOutputCoins instead of subUnlockedCoins, though. Sanctions are a check on the sender(s) and quarantine is a check/change on the receiver(s), but also need to know the sender(s). But for performance reasons, we probably don't want to make SendCoins call InputOutputCoins, and InputOutputCoins logically can't be refactored to use SendCoins. So if injecting generic restrictions, there'd still be two things to inject. Plus, in the case of quarantine, it's not a simple "If thing, then error" check.

[SpicyLemon]

With this change: #12610, I think it'd be possible to have a single injectable function in the x/bank module to facilitate both of these needs. It'd be some sort of "restriction" function that'd take in a to address, from address and coins amount, and output the desired to address and an error.

It'd be something like this:

type Restriction func(fromAddr, toAddr sdk.AccAddress, amount sdk.Coins) (toAddr sdk.AccAddress, err error)

There'd need to be some way to inject that into the x/bank module so that the send keeper can use it. During a SendCoins, there'd just be the one call to the restriction. During an InputOutput coins, it'd be called once for each output.

[SpicyLemon]

I created #14224 to allow injection of restrictions on sends that would allow for quarantine and/or sanction modules (or whatever) to provide their needed restrictions to the bank module.

The actual quarantine and sanction modules can then come from outside the SDK.

[alexanderbez]

We should ask in the community call if other app devs and chains would find this useful. I'm open to the changes, just want to make sure there is buy-in prior to breaking APIs and such.

[SpicyLemon]

We should ask in the community call if other app devs and chains would find this useful. I'm open to the changes, just want to make sure there is buy-in prior to breaking APIs and such.

I'd love to discuss it. Can I please get it added to the agenda, and can I also please get an invite to the call (either to this account ([email protected]), or [email protected])?

[alexanderbez]

Done and done!

[JimLarson]

From today's community call, the changes contemplated in #14271 ("Refactor besting into general encumbrances on accounts") overlaps somewhat. Since we want to avoid providing multiple APIs for doing the same thing, let's see if there's a way to subsume one in the other, or provide clear distinction. The encumbrances proposal would provide a way to keep a certain amount locked up in the account, though it could be also unvested, used for staking, etc.

[robert-zaremba]

Correct Quarantine implementation will require writing an IBC ICS20 middleware.

[JimLarson]

Repeating the comments I left in #14224, the mechanism proposed here plays nicely with the changes contemplated in #14271 - the disjunctive liabilities can be expressed as a restriction (or as one for each dimension). I suggested that the PR could also refactor subUnlockedCoins() into a restriction check and an unconditional subCoins() for nice symmetry with addCoins(), for an overall simplification of the code.

[SpicyLemon]

I'm going to close this issue since it's probably not something to add to the SDK at this time. At the very least, these modules are example use cases of restrictions/hooks needed on bank transfers.

The Provenance fork of the SDK has these modules, but they'd would be more portable if the SDK added injectable restrictions (#14224). If that PR isn't used, hopefully, the discussions in #14701 result in something these modules can use.

[tac0turtle]

Reopening as we haven't had a chance to triage this properly. Sorry for the long delay