Add describe-guard primitive #851
Conversation
There was a problem hiding this comment.
Great job @mightybyte !
In addition to comments, we just need some basic exercising of the code in tests/pact/caps.repl -- you'll see a ton of guard stuff in there you can grab. The model would just be
(expect "describe-guard for keyset"
{ 'type: "keyset", 'keyset: (read-keyset 'k) }
(describe-guard (read-keyset 'k)))
etc
| GKeySet ks -> | ||
| computeGas' gas i (GUserApp Defun) $ | ||
| return $ toTObject TyAny def | ||
| [ ("type", tStr "KeySet") |
There was a problem hiding this comment.
How should we handle type-specific metadata? I'm guessing we should only have type be common and everything else varies, but for some that's JSON no-no. What about a details field which is a varying object? Or is that overthinking it
There was a problem hiding this comment.
The main motivating purpose for this was providing information to apps / wallets / etc about what keys they should sign with. So the big thing my immediate use case wants from this is public keys. Beyond that I really don't know. Probably the most important criteria is that any changes we make down the road can be done in a backwards-compatible manner.
There was a problem hiding this comment.
I guess one other high level question is whether we should put the case-specific data at the same level or if we should make some kind of generic object wrapper to make it easier to decode.
There was a problem hiding this comment.
case-specific data at the same level or if we should make some kind of generic object wrapper to make it easier to decode
That was what I was trying to suggest with details, let's do that.
src/Pact/Native/Keysets.hs
Outdated
| GModule _ -> computeGas' gas i (GUserApp Defun) $ | ||
| return $ toTObject TyAny def [ ("type", tStr "Module") ] | ||
| GUser _ -> computeGas' gas i (GUserApp Defun) $ | ||
| return $ toTObject TyAny def [ ("type", tStr "User") ] |
There was a problem hiding this comment.
OK this is where it gets interesting.
First, it should have fun and args from UserGuard.
(Note that all of these (including other types) should probably try to stay consistent with JSON field names. This is maybe why a details field could be good, to make it really clear that we're enumerating the constructor JSON-style)
Second: wasn't it a motivation to show all keysets? This is where we might be able to traverse the guard structure. Let's look at the JSON of a gas station with a key:
{ "fun": "util.guards1.enforce-guard-any"
"args": [
[ { "pred": "keys-all",
"keys": [ "9a4849687cbcfeb1f7c6510539638da576289508aedcc75f4d6ad3ed2623635c" ]
},
{ "fun": "util.guards1.enforce-guard-all",
"args": [
[ { "fun": "coin.gas-only",
"args": []
},
{ "fun": "util.guards1.enforce-below-or-at-gas-price",
"args": [ 1e-08 ]
},
{ "fun": "util.guards1.enforce-below-or-at-gas-limit",
"args": [ { "int": 10000 } ]
}
] ]
}
] ]
}
So, first, it might be nice to try to pretty-print in a desc field. More importantly, we can walk the args in that any time we find a TGuard argument, we can find keysets! Then we dump them in a keysets field. What do you think?
There was a problem hiding this comment.
This sounds promising! I'm also thinking about the scenario where maybe you have a smart wallet that allows different keysets in different situations. Is there a way the smart wallet module could somehow expose what keys it's accepting? I'm imagining situations where it's not necessarily static. For instance, maybe you have a smart wallet designed to make funds available to beneficiaries on the event of death or loss of owner's keys. In this situation you might have a time delay. If it's been less than a year since the last transaction, only allow the owner's key. Otherwise allow some multi-sig of social recovery with maybe a family member plus an attorney or something.
Co-authored-by: Stuart Popejoy <8353613+sirlensalot@users.noreply.github.com>
Co-authored-by: Stuart Popejoy <8353613+sirlensalot@users.noreply.github.com>
| @@ -49,6 +53,14 @@ keyDefs = | |||
| "Define keyset as NAME with KEYSET, or if unspecified, read NAME from message payload as keyset, \ | |||
| \similarly to 'read-keyset'. \ | |||
| \If keyset NAME already exists, keyset will be enforced before updating to new value." | |||
| ,setTopLevelOnly $ defGasRNative "describe-keyset" descKeySet | |||
| (funType tTyObjectAny [("keyset",tTyString)]) | |||
| ["(env-data {\"k\": { \"pred\": \"keys-all\", \"keys\": [\"abc\"] }) (describe-keyset (read-keyset 'k))"] | |||
There was a problem hiding this comment.
Not sure why this example is failing the test suite.
There was a problem hiding this comment.
Here's working code (will need quote escapes):
(env-data {"k": ["abc"] }) (define-keyset 'k (read-keyset 'k)) (describe-keyset 'k)
|
@sirlensalot This feature never landed. Do we still want this? |
Also moves
describe-keysetto the "Keysets" section, which seems more intuitive.