Standard for index discovery from ledgers #106
Conversation
bogwar
changed the title
|
|
||
| ## 3. Index Canister Interface | ||
|
|
||
| The index canister associated with the ledger SHOULD implement the following minimal Candid interface: |
There was a problem hiding this comment.
What's the semantic difference between SHOULD and MUST in this document?
There was a problem hiding this comment.
MUST is more stringent than SHOULD -- see RFC2119
| ## 4. Implementation Considerations | ||
|
|
||
| Implementers should ensure that: | ||
| - The `icrc106:index_principal` metadata entry accurately reflects the principal of the associated index canister. |
There was a problem hiding this comment.
This kind of implies that the index should be deployed before the ledger, right?
But then the suggested Index API that has
ledger_id: () -> (principal) query;
makes it impossible to comply with this standard in the state in which Index is already deployed but the Ledger isn't.
Should we maybe consider:
ledger_id: () -> (record { opt principal }) query;
or maybe
ledger_id: () -> (LedgerIdResult) query;
?
There was a problem hiding this comment.
The metadata could be updated after the index is deployed, at which point the principal of the index would be known, no?
There was a problem hiding this comment.
Yes. I'm just pointing out that this requirement would be temporarily violated before both are deployed.
I don't see any severe practical issues w.r.t. this, though.
| num_blocks_synced : BlockIndex; | ||
| }; | ||
|
|
||
| service : { |
There was a problem hiding this comment.
I wonder what is the reason that some standardized functions are called with the standard in the prefix (e.g., icrc2_transfer_from), while others are not.
Probably this is used when it's necessary to disambiguate new functions from priorly existing ones.
I wonder if it would be a better convention to always have standard functions prefixed with the standard name.
There was a problem hiding this comment.
The index interface doesn't follow a standard, so the methods don't have this name spacing
There was a problem hiding this comment.
Then I wonder, should there be a note saying explicitly which canisters in the Ledger suite this standard is specifying? I now suppose it's the Ledger itself, and not the Index.
| service : { | ||
| get_account_transactions: (GetAccountTransactionsArgs) -> (GetTransactionsResult) query; | ||
| ledger_id: () -> (principal) query; | ||
| list_subaccounts : (ListSubaccountsArgs) -> (vec SubAccount) query; |
There was a problem hiding this comment.
The ListSubaccountsArgs type takes start, but how does the client know how to get subsequent pages? I guess they should set start to the last value form the privious response.
Should this be documented in the https://github.com/dfinity/ICRC/blob/43d368ba79d30e850a422fd693b827b528c26ff1/ICRCs/ICRC-106/ICRC-106.md#list_subaccounts section?
|
|
||
| type Transaction = record { | ||
| burn : opt Burn; | ||
| kind : text; |
There was a problem hiding this comment.
Imho, this field is redundant, since the kind can be deduced from the optional fields.
A slightly better way to encode this would be to have the specific kinds exclusive. Right now, a valid instance of this type could be both mint and burn, right?
There was a problem hiding this comment.
Here we're just pinning the current interface of the index canisters, so for completeness we include all of the record fields
| kind : text; | ||
| mint : opt Mint; | ||
| approve : opt Approve; | ||
| timestamp : nat64; |
There was a problem hiding this comment.
| timestamp : nat64; | |
| timestamp_seconds : nat64; |
| type Burn = record { | ||
| from : Account; | ||
| memo : opt vec nat8; | ||
| created_at_time : opt nat64; |
There was a problem hiding this comment.
| created_at_time : opt nat64; | |
| created_at_timestamp_seconds : opt nat64; |
| fee : opt nat; | ||
| from : Account; | ||
| memo : opt vec nat8; | ||
| created_at_time : opt nat64; |
There was a problem hiding this comment.
| created_at_time : opt nat64; | |
| created_at_timestamp_seconds : opt nat64; |
| fee : opt nat; | ||
| from : Account; | ||
| memo : opt vec nat8; | ||
| created_at_time : opt nat64; |
There was a problem hiding this comment.
| created_at_time : opt nat64; | |
| created_at_timestamp_seconds : opt nat64; |
| created_at_time : opt nat64; | ||
| amount : nat; | ||
| expected_allowance : opt nat; | ||
| expires_at : opt nat64; |
There was a problem hiding this comment.
| expires_at : opt nat64; | |
| expires_at_timestamp_seconds : opt nat64; |
|
|
||
| type Transfer = record { | ||
| to : Account; | ||
| fee : opt nat; |
There was a problem hiding this comment.
| fee : opt nat; | |
| fee : opt Tokens; |
| from : Account; | ||
| memo : opt vec nat8; | ||
| created_at_time : opt nat64; | ||
| amount : nat; |
There was a problem hiding this comment.
| amount : nat; | |
| amount : Tokens; |
| }; | ||
|
|
||
| type Approve = record { | ||
| fee : opt nat; |
There was a problem hiding this comment.
| fee : opt nat; | |
| fee : opt Tokens; |
| type Approve = record { | ||
| fee : opt nat; | ||
| from : Account; | ||
| memo : opt vec nat8; |
There was a problem hiding this comment.
Curious, why not
| memo : opt vec nat8; | |
| memo : opt blob; |
| from : Account; | ||
| memo : opt vec nat8; | ||
| created_at_time : opt nat64; | ||
| amount : nat; |
There was a problem hiding this comment.
| amount : nat; | |
| amount : Tokens; |
| memo : opt vec nat8; | ||
| created_at_time : opt nat64; | ||
| amount : nat; | ||
| expected_allowance : opt nat; |
There was a problem hiding this comment.
| expected_allowance : opt nat; | |
| expected_allowance : opt Tokens; |
Co-authored-by: Arshavir Ter-Gabrielyan <[email protected]>
Co-authored-by: Arshavir Ter-Gabrielyan <[email protected]>
Co-authored-by: Arshavir Ter-Gabrielyan <[email protected]>
|
LGTM! |
A standard to allow for the discovery of the index canister from the corresponding ledger. This standard will exist in draft form only since we will work on a proper ICRC standard for index canisters (which will include discoverability).
A secondary goal for the standard is to document the interface of the current index canister.