AA-enabled Identity#1827
Conversation
Simplify the construction of an off-chain copy of an `IdentityV2` through a single `move_view_call` that returns its three components: - did document metadata; - configuration; - legacy ID (if any);
| #[error(code = 6)] | ||
| const ETransactionDigestMismatch: vector<u8> = b"Transaction digest mismatch"; | ||
| #[error(code = 7)] | ||
| const EInsufficientPermissions: vector<u8> = b"Controller does not have sufficient permissions"; |
There was a problem hiding this comment.
Would it make sense to split this error into EInsufficientPermissions and EInsufficientWeight? Both values behave differently, and knowing which of them is off might help to find out why an action was rejected.
| (permissions & other_permissions.bitwise_not()) == 0 | ||
| } | ||
|
|
||
| public macro fun can_propose_tx(): u64 { |
There was a problem hiding this comment.
Readability wise, would it make sense to move the permission related components into their own file? IMO permission.can_propose_tx() might be clearer than the current config.can_propose_tx() (initially thought, it would check the current config, whether the caller can propose a tx or something and not return the value for such a check >.<).
While on that topic already (^^;;), would have expected a function starting with can_ to return a boolean value. Maybe we could drop the can_ to give it a more "enum-like" feeling, resulting in permission.propose_tx(). This would also fit the existing admin() function naming.
|
|
||
| public macro fun admin(): u64 { | ||
| 1 << 63 | ||
| } |
There was a problem hiding this comment.
Was wondering if we should add something like all_permissions, that returns std::u64::max_value!()). Might be used in the migration and the v2 new.
| validate_remove_controller_call(&move_call, inputs, config, permissions, weight); | ||
| } else if (move_call_is(&move_call, b"update_threshold")) { | ||
| assert_permissions(permissions, config::can_update_threshold!()); | ||
| } else {} |
There was a problem hiding this comment.
Putting a reminder here, to revisit the question about whether we should add permission checks for executing transactions with the identity, e.g. control all owned assets or having all controllers being allowed to call all functions of all "owned" (actually owned by the identity or the identity being allowed to do something in a shared object) objects of the identity.
| cap.put_back(token, borrow); | ||
| } | ||
|
|
||
| public fun execute_aa_migration( |
There was a problem hiding this comment.
(Reminder) What about the objects owned by the identity?
IIRC, we can't just provide them as arguments here, so this should be responsibility of the Rust API, right?
Should work, but a check or something here would be really nice, but I don't think, that's an option. >.<
| // - Controller A: address 0x1, weight 1; | ||
| // - Controller B: address 0x2, weight 1; | ||
| // - Controller C: address 0x3, weight 2; | ||
| fun make_identity(clock: &Clock, ctx: &mut TxContext) { |
There was a problem hiding this comment.
Maybe? ^^
| fun make_identity(clock: &Clock, ctx: &mut TxContext) { | |
| /// Creates an identity with 3 controllers threshold is 2: | |
| /// - Controller A: address 0x1, weight 1; | |
| /// - Controller B: address 0x2, weight 1; | |
| /// - Controller C: address 0x3, weight 2; |
| addresses: vector<address>, | ||
| weights: vector<u64>, | ||
| permissions: vector<u64>, | ||
| threshold: u64, |
There was a problem hiding this comment.
Not sure if we actually need this, but wanted to pin it here before I forget it. :D
Would it make sense to use separate thresholds per permission? But then again, this would probably require permission based weights as well.
This might allow distributing responsibilities and powers better, and for example deleting a DID might require a higher threshold, where all members must agree with the same weights, while updating DID documents could be fastlaned by responsible parties.
But it would complicate the API for creating identities and adding/updating controllers. One way would be to add a default threshold/weight and a map to override the values for both functions. E.g. call could provide a default threshold of 2, but require a much higher weight for deleting the identity. Similarly, a controller could have a default weight of 1, and a weight of 5 for the responsibility to update the DID document.
Just as mentioned, not sure if we need this, but wanted to chip better sooner than later, as adding it might require a separate migration. >.<
| } | ||
| } | ||
|
|
||
| public fun delete_controller_cap(self: &AAMigrationRegistry, cap: ControllerCap) { |
There was a problem hiding this comment.
Is this function only used in tests?
2 participants
No description provided.