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

Allow restricted coins stuck in quarantine to be released. #1654

Open
4 tasks
SpicyLemon opened this issue Aug 18, 2023 · 0 comments
Open
4 tasks

Allow restricted coins stuck in quarantine to be released. #1654

SpicyLemon opened this issue Aug 18, 2023 · 0 comments
Labels
Milestone

Comments

@SpicyLemon
Copy link
Contributor

Summary

Provide a way to move restricted coins out of the quarantined-funds-holder account that doesn't break things.

Problem Definition

Funds can get stuck in quarantine in some circumstances.

For example, say chickens are a restricted coin.
Alice has transfer permission on them, but they also have a required attribute: cluck.provenance.pb.
Bob has opted into quarantine and does not auto-accept from anyone.
Bob does NOT have the cluck.provenance.pb attribute.

When Alice sends Bob 5chickens, the transfer will be allowed because Alice has transfer permission.
However, because Bob's account is quarantined, those chickens will get sent to the QFH (Quarantined-Funds-Holder) account, and a quarantine record is made of them.

When Bob sends an Accept to the quarantine module, it will fail because he does not have the required attribute.
Alice cannot move them out of the QFH either (using a MsgTransferRequest) because that's a module account.

The funds are effectively stuck there until either:

  1. Bob is given the required attribute.
  2. The QFH is given transfer permission.

Giving the QFH transfer permission is probably not a wise choice, and there might be a good reason why Bob doesn't have that attribute.

So, we need a way for someone with transfer authority to get those funds out of the QFH, ideally to their intended target, but possibly to a different target.

Proposal

Give the marker keeper knowledge of the quarantine keeper.

This would probably be best handled using a WithQuarantineKeeper add-on-function on the marker keeper. In app.go, after creating the quarantine keeper, update the app's marker keeper to be one with the provided quarantine keeper.

An alternative approach is to have a SetQuarantineKeeper that updates a field in the marker keeper that is a holder, holding a pointer the quarantine keeper. This is similar to how the SendRestrictionFns are stored in the bank module.

In TransferCoins, if the from is the QFH, get the quarantine records (using GetQuarantineRecords) and make sure they've got enough to cover this transfer. If so, update them appropriately and allow the transfer to proceed. This should only be possible if to is no longer quarantined, or a usable authz authorization exists with granter = to, grantee = admin, and msg type = for quarantine's MsgAccept. There's no need to bypass the quarantine SendRestrictionFn because it bypasses itself when the from is the QFH.

We only want to be able to transfer from the QFH if the to address wants to allow the coins in their account. An attempt to force transfer funds from any other account to a quarantined account will result in the funds going to QFH. We shouldn't then allow another forced transfer to pull those out of QFH to go to the quarantined address unless the quarantined address says it's okay.

Also, create a new ReclaimQuarantine endpoint in the marker module. It will take in addresses for original_to, and admin, and have a Denom field. The admin will need transfer (and/or deposit permission?). Use IterateQuarantineRecords to get all quarantine records to the original_to address that have the coins of the provided Denom. Send all of that from QFH to the marker account and update the quarantine records appropriately.

We want to be able to reclaim quarantined funds in the following circumstances:

  • The to is missing a required attribute, and will never get it (unknowable by the blockchain).
  • The to is an account that no one has ever tried to claim (sequence = 0 and may or may not have a public key).
  • The to is an account that lost their key (sequence != 0 and probably has a public key).

Here's some situations where it'd be rude to reclaim quarantined funds:

  • The account is in use and the marker doesn't have any required attributes (i.e. an Accept would work just fine).
  • The account doesn't have the required attributes, but expects to get them soon.

Basically, I'm not exactly sure what restrictions can be placed on reclaiming quarantined funds. We probably want to only allow it for markers with forced transfer available, but we might not.


When updating quarantine records, if the resulting record has .Coins.IsZero() == true, move all its unaccepted addresses to accepted. If there's still coins left, though, do not alter the record's accepted/unaccepted addresses. The SetQuarantineRecord function deletes the record if it's fully accepted, but would write one that isn't fully accepted, even if it doesn't have any .Coins. That's why we mark such entries as fully accepted before giving them to SetQuarantineRecord. If they have other funds in them, we just need to tell it about the new reduced amount, but don't want to alter their state of acceptance.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

2 participants