-
Notifications
You must be signed in to change notification settings - Fork 1
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
Delegated Opt In Review #1
base: review
Are you sure you want to change the base?
Changes from 5 commits
fa5b4e0
ce6d6b7
ee9ab06
80cd20e
6f4f322
76ca460
d4b111a
978b12f
0dadbfe
9169a14
962de7a
76e7d34
f45b476
3bb98bd
c5c2d95
b69d790
89e1515
7a559c5
c5d7203
4701fd7
75ceab2
56c7aa0
5eb33dd
bf072e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
## Abstract | ||
This ARC contains is an implementation of [ARCX](https://github.com/algorandfoundation/ARCs/pull/229). The goal is to provide users a way to delegate asset opt-ins while having a high degree of control compared to a standalone logic signature without an application. The expectation is that a single instance of this ARC will be deployed on Algorand networks. | ||
|
||
## Motivation | ||
[ARCX](https://github.com/algorandfoundation/ARCs/pull/229) provides a standard for delegated asset opt-ins, but there are some UX problems that need to be addressed. First, there needs to be a way to control how long the signature is valid for. Without an application to control this, signing the logic signature program would be irreversible. There also needs to be a standardized way to store and read signatures for a given account so dApps and other users can take advantaged of delegated opt-ins. | ||
|
||
## Specification | ||
This is an implementation of [ARCX](https://github.com/algorandfoundation/ARCs/pull/229). There is additional functionality provided by the application in this ARC in the methods of the application. | ||
|
||
For all methods, refer to the [ABI JSON description](./contracts/artifacts/DelegatedOptIn.abi.json) for complete descriptions of arguments. | ||
|
||
### Signature Storage | ||
|
||
`setOpenOptInSignature(byte[64],address,txn)void` and `setAddressOptInSignature(byte[64],address,address,txn)void` are methods for adding signatures to box storage for open opt-ins and address opt-ins, respectively. | ||
|
||
Both methods utilize [the verifier lsig](./contracts/verifier_lsig.teal) to verify the address being added to box storage matches the address(es) used as the key. This means anyone can query the applications boxes to get the signature for the given address(es) and be certain that it is correct. | ||
|
||
### End Times | ||
|
||
`setOpenOptInEndTime(uint64)void` and `setAddressOptInEndTime(uint64,address)void` are methods for setting the end time of a signature for open opt-ins and address opt-ins, respectively. An endtime signifies when the delegated logic signature will no longer work. The time corresponds to the epoch time (seconds) returned by `global LatestTimestamp`. End times can be updated at any time to any value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does setting the endtime to 0 delete the box? Does the MBR get recovered somehow? Actually, I'm assuming that the non-existence of an endtime means there is no endtime? So perhaps setting the endtime to MaxUint64 should delete the box and recover the MBR. I suppose that really I'm asking for a way to delete all of the storage and have it mean "I want out of this whole business, give me my MBR back". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now there is no MBR recovery mechanism. I figured it added extra complexity for such a small amount of ALGO it wasn't necessarily worth it. Open to the idea of MBR recovery though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm doubling down on my dislike of storing logicsigs on chain without a deletion mechanism. If I say I want to allow someone to opt me in, I should just be able to remove that. I suppose I'm really confused by the specific sender mechanism. What's the use case that makes it worth enshrining this idea? Why not just "allow all, deny all"? (and by that I mean let's also scrap endtime. just allow revocation.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, now I see. You are just storing the signature. That seems very reasonable. But it also means that using one box for the signature and the endtime is easy. They are both fixed size. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming we still want to maintain the current design goals, we actually couldn't make it one box. The signatures are stored per the resepective public key, but the end times are stored per address. This means one can have multiple accounts rekeyed to a single auth account and have different rules set for each one. For example, I could have a single hardware wallet that is the auth addr for a "DeFi Account" and an "NFT Account". The "DeFi Account" delegates opt-ins to DeFi apps and "NFT Account" delegates opt-ins to NFT marketplaces, but not the other way around. This can currently be done because I can set the end times to be unique for each account. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should mention the downside of this approach is it makes fresh rekeyed accounts open to any opt-ins before setting the end times, but in general I think this is reasonable tradeoff for having per-account control. Open to other opinions though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think it's probably a mistake to have this separation. Today, rekeying two accounts to the same key means that they are under the exact same authority. It seems like a bad idea to introduce a mechanism that changes that model. If this becomes popular, we then have two ways of thinking about rekeying, and users must understand both. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah true that's a really good point. Removing this means we can just use the existence of the signature boxes to signify whether delegated opt ins are permitted or not. We can also remove the time component and if people really want an automated disable, that can be done at a higher level. |
||
|
||
### Opt-Ins | ||
|
||
`openOptIn(pay,axfer)void` and `addressOptIn(pay,axfer)void` are implementations of the [ARCX](https://github.com/algorandfoundation/ARCs/pull/229) interfaces. They both verify the MBR payment is sent to the account opting in and that it covers the ASA minimum balance requirement, which is stored in global storage. It also verifies the value of `global LatestTimestamp` is less than the set end time for the account opting in. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By aware, though I don't think it matters, that So if the last block was at 9998, and the end time is 10000, but the next block gets added at 10002, then you can have an opt-in the occurs after the set endtime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that's why I specified the usage of |
||
|
||
### Storage | ||
|
||
| Type | Key | Description | | ||
| ---- | --- | ----------- | | ||
| Global | "sigVerificationAddress" | Stores the address of [the verifier lsig](./contracts/verifier_lsig.teal) | | ||
| Global | "assetMBR" | Stores the ASA MBR | | ||
| Box | `auth-addr` | Mapping of signer to open opt-in signature | | ||
| Box | "e-" + `auth-addr` | Mapping of signer to open opt-in end time | | ||
| Box | `(signer,sender)` | Mapping of the hash of the signer address plus sender address to address opt-in signature | | ||
| Box | "e-" + `sha256(signer,sender)` | Mapping of the hash of the signer address plus sender address to address opt-in end time | | ||
|
||
## Rationale | ||
Box storage is used to store signatures indefinitely and [the verifier lsig](./contracts/verifier_lsig.teal) ensures the signature is always correct. | ||
|
||
End time functionality is provided to allow users to revert the effects of signing the logic signature programs without having to rekey. It also lets users open their account for opt-in delegations for a short amount of time without having to remember to manually undo it. | ||
joe-p marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Asset MBR is stored in global storage in case the MBR for asset were to ever change. | ||
|
||
## Backwards Compatibility | ||
N/A | ||
|
||
## Test Cases | ||
Tests written with Jest and algokit for the contract and logic signatures can be seen in [test.ts](./tests/test.ts). | ||
|
||
``` | ||
Delegated Opt In App | ||
create | ||
✓ creates the app (1427 ms) | ||
setSigVerificationAddress | ||
✓ works with valid address (939 ms) | ||
setOpenOptInSignature | ||
✓ works with valid signature and lsig (1031 ms) | ||
openOptIn | ||
✓ works with valid lsig and method call (973 ms) | ||
setOpenOptInEndTime | ||
✓ works with 0xffffffff as the end time (2091 ms) | ||
✓ works with 0 as the end time (1137 ms) | ||
setAddressOptInSignature | ||
✓ works with valid signature and lsig (926 ms) | ||
addressoptOptIn | ||
✓ works with valid lsig, method call, and sender (914 ms) | ||
setAddressOptInEndtime | ||
✓ works with 0xffffffff as the end time (1001 ms) | ||
✓ works with 0 as the end time (1611 ms) | ||
``` | ||
|
||
## Reference Implementation | ||
[delegated_optin_app.algo.ts](./contracts/delegated_optin_app.algo.ts) is the application written in TEALScript. | ||
|
||
[DelegatedOptIn.approval.teal](./contracts/artifacts/DelegatedOptIn.approval.teal) is the TEAL compiled from TEALScript. | ||
|
||
## Security Considerations | ||
|
||
The test cases test proper functionality of all methods, but there has been no extended effort in attempt to break the contract. Most of the functionality in the app and logic signatures is relatively simple, so the chances of unexpected bugs is relatively low. | ||
|
||
It should be made clear that signatures are stored mapped to the signer (`auth-addr`) and end times are mapped to the public Algorand address. This means when a signature is made known, every account with the signing account as the `auth-addr` can now be opted-in to assets via the delegated logic signature. Every account must set the end times to their desire, because without an end time set the signature will work regardless of the latest timestamp. | ||
joe-p marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Copyright | ||
Copyright and related rights waived via <a href="https://creativecommons.org/publicdomain/zero/1.0/">CCO</a>. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
#pragma version 8 | ||
#define MasterAppCall load 0 | ||
|
||
// Save MasterAppCall | ||
txn GroupIndex | ||
int 1 | ||
+ | ||
store 0 | ||
|
||
// Verify amount is 0 | ||
txn AssetAmount | ||
int 0 | ||
== | ||
assert | ||
|
||
// Verify sender == receiver | ||
txn AssetReceiver | ||
txn Sender | ||
== | ||
assert | ||
joe-p marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Verify fee is 0 (covered by sender) | ||
txn Fee | ||
int 0 | ||
== | ||
assert | ||
|
||
// Verify assetCloseTo is not set | ||
txn AssetCloseTo | ||
global ZeroAddress | ||
== | ||
assert | ||
|
||
// Verify called atomically with master app | ||
MasterAppCall | ||
gtxns ApplicationID | ||
int TMPL_DELEGATED_OPTIN_APP_ID | ||
== | ||
assert | ||
|
||
// Verify the correct method is being called | ||
MasterAppCall | ||
gtxnsa ApplicationArgs 0 | ||
method "addressOptIn(pay,axfer)void" | ||
== | ||
assert | ||
|
||
// Verify the sender is the correct address | ||
MasterAppCall | ||
gtxns Sender | ||
addr TMPL_AUTHORIZED_ADDRESS | ||
== |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
{ | ||
"name": "DelegatedOptIn", | ||
"desc": "", | ||
"methods": [ | ||
{ | ||
"name": "setSigVerificationAddress", | ||
"args": [ | ||
{ | ||
"name": "lsig", | ||
"type": "address", | ||
"desc": "The address of the verifier lsig" | ||
} | ||
], | ||
"desc": "Set the address of the verifier lsig. This will only be called once after creation.", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "updateAssetMBR", | ||
"args": [ | ||
{ | ||
"name": "asset", | ||
"type": "asset", | ||
"desc": "The asset to opt into and opt out of to determine MBR" | ||
} | ||
], | ||
"desc": "Updates the asset MBR", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "setOpenOptInSignature", | ||
"args": [ | ||
{ | ||
"name": "sig", | ||
"type": "byte[64]", | ||
"desc": "The signature of the lsig" | ||
}, | ||
{ | ||
"name": "signer", | ||
"type": "address", | ||
"desc": "The public key corresponding to the signature" | ||
}, | ||
{ | ||
"name": "verifier", | ||
"type": "txn", | ||
"desc": "A txn from the verifier lsig to verify the signature" | ||
} | ||
], | ||
"desc": "Set the signature of the lsig for the given account", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "openOptIn", | ||
"args": [ | ||
{ | ||
"name": "mbrPayment", | ||
"type": "pay", | ||
"desc": "Payment to the receiver that covers the ASA MBR" | ||
}, | ||
{ | ||
"name": "optIn", | ||
"type": "axfer", | ||
"desc": "The opt in transaction, presumably from the open opt-in lsig" | ||
} | ||
], | ||
"desc": "Verifies that the opt in is allowed", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "setOpenOptInEndTime", | ||
"args": [ | ||
{ | ||
"name": "timestamp", | ||
"type": "uint64", | ||
"desc": "After this time, opt ins will no longer be allowed" | ||
} | ||
], | ||
"desc": "Set the timestamp until which the account allows opt ins", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "setAddressOptInSignature", | ||
"args": [ | ||
{ | ||
"name": "sig", | ||
"type": "byte[64]", | ||
"desc": "The signature of the lsig" | ||
}, | ||
{ | ||
"name": "signer", | ||
"type": "address", | ||
"desc": "The public key corresponding to the signature" | ||
}, | ||
{ | ||
"name": "allowedAddress", | ||
"type": "address", | ||
"desc": "" | ||
}, | ||
{ | ||
"name": "verifier", | ||
"type": "txn", | ||
"desc": "" | ||
} | ||
], | ||
"desc": "Set the signature of the lsig for the given account", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "addressOptIn", | ||
"args": [ | ||
{ | ||
"name": "mbrPayment", | ||
"type": "pay", | ||
"desc": "Payment to the receiver that covers the ASA MBR" | ||
}, | ||
{ | ||
"name": "optIn", | ||
"type": "axfer", | ||
"desc": "The opt in transaction, presumably from the address opt-in lsig" | ||
} | ||
], | ||
"desc": "Verifies that the opt in is allowed from the sender", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
}, | ||
{ | ||
"name": "setAddressOptInEndTime", | ||
"args": [ | ||
{ | ||
"name": "timestamp", | ||
"type": "uint64", | ||
"desc": "After this time, opt ins will no longer be allowed" | ||
}, | ||
{ | ||
"name": "allowedAddress", | ||
"type": "address", | ||
"desc": "The address to set the end time for" | ||
} | ||
], | ||
"desc": "Set the timestamp until which the account allows opt ins for a specific address", | ||
"returns": { | ||
"type": "void", | ||
"desc": "" | ||
} | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see why this is a nice, useful app to use in combination with the ARC-50, but I don't see why it should be considered "special". The point of ARC-50 was to give users control - let them have an app that they trust to check opt-ins for them. This is one of them, but if it's the only one, then the flexibility that ARC-50 promises is reduced to the flexibility that this particular implementation provides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the idea isn't that this is the only ARC50 implementation, but it's a standardized one that every wallet/dApp in the ecosystem can integrate. Other people can make whatever implementations they want, but the more implementations there are the more fragmented the ecosystem can get.