Automatically approve built-in market as operator for all minted datacap #611
Conversation
There was a problem hiding this comment.
For now this looks ok. I'm still stuck on understanding how allowances change when funds are transferred and burned. I hope we can have the market track ups and downs in allowance as datacap is minted to and transferred out of clients but I don't see and decrease allowance calls yet. Maybe this is happening already inside the token library? If we're just recording the high water mark it would achieve the same thing and be clearer to give the market an effectively infinite allowance from the start.
| operator_data: Default::default(), | ||
| token_data: Default::default(), | ||
| }; | ||
| rt.expect_send( |
There was a problem hiding this comment.
I might be confused about this but I don't think this ExitCode::OK is a good representation of what will actually happen. The receiver hook gets called on the verified client which for the time being will be an actor that can sign things which means it will not implement the receiver hook method and will fail with INVALID_METHOD or something like that.
I think that everything still works because of your impl of Token Messaging trait and I want to see that exercised.
There was a problem hiding this comment.
the verified client which for the time being will be an actor that can sign things which means it will not implement the receiver hook method
Your confusion is that the built-in account and multisig actors do implement the receiver hook (#555), which is independent of any signing/validation-related things.
|
Ok this is starting to make more sense. I think the market actor is going to be calling transfer_from where the current code calls UseBytes and in this case the allowance will be used. If this is the only movement of datacap from verified clients we get the invariant Market Allowance == Verified Client Balance. Taking into account burns, transitive burns through destroy, and direct transfers which are all allowed we have the weaker but still safe Market Allowance >= Verified Client Balance. What I don't quite see is how this works when we open up FIL+ for use with general markets. Are you imagining all FIL+ registered markets get allowance for all verified clients and it doesn't matter that we're way overcounting? Is that the purpose of the operator list rather than just hardcoding market in datacap or sending a single value? |
anorth
force-pushed
the
anorth/marketdelegate
branch
from
6076bcf to
a884130
Compare
|
Thanks for challenging. You're right that this was only effectively setting a high water mark allowance. So to simplify, I've changed it to a fixed 2bn allowance. (It's actually not fixed right now, but we can do so when the token library lets us, which I've requested).
For general markets, clients will need to explicitly approve allowance for those markets. I'd like to apply this to the built-in one some day too. Market UIs should generally make that transparent for users. The operator list is just a way to move this policy into verifreg rather than the datacap actor. It could have been a single value, but then one might want it to be optional, at which point a list just seemed like a more natural representation. |
Codecov Report
@@ Coverage Diff @@
## decouple-fil+ #611 +/- ##
================================================
Coverage ? 84.06%
================================================
Files ? 95
Lines ? 18728
Branches ? 0
================================================
Hits ? 15743
Misses ? 2985
Partials ? 0 |
Minting is not a standard token method, so adding a parameter to mint allows this to happen with the business logic of which account to approve encapsulated in the verified registry, but no additional inter-actor sends.
Closes #529.