Skip to content
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

list allowances standard initial commit #191

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

list allowances standard initial commit #191

wants to merge 8 commits into from

Conversation

bogwar
Copy link
Contributor

@bogwar bogwar commented Jul 24, 2024

This is a draft for the ICRC-191 standard for listing outstanding allowances on ICP ledgers

Copy link

@skilesare skilesare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few spelling or technical things(like the number)...one philosophical difference, but it likely fine either way.

}
```

The endpoint returns up to `taken` allowances of the from_account.owner, starting with the allowance between `from_account` and `to_account`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it

|:------:|
|Draft|

# ICRC-191: Enhanced Allowance Query Mechanism with Pagination

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typically the number comes from github.com/dfinity/icrc and not from the icrc-1 repo...I think we're around about 101 or 102.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved it there: got number 103


Outstanding allowances, as specified in the ICRC-2 standard, are represented as a map from pairs of accounts to allowances. To specify the behavior of `icrc191_list_allowances`, the set of pairs `(Account, Account)` is ordered lexicographically. Let `first_principal` be the lexicographically first principal, and `first_subaccount` be the lexicographically first subaccount (the default subaccount, i.e., the all-0 32-byte string). Let `caller_principal` be the principal of the caller.

The `icrc191_list_allwances` method behaves as follows:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling


Then:

- If `p0` calls the list allowances endpoint, with `from_account = (p0, s0)`, `prev_spender = None` and `take=4` the endpoint returns ``(A1, A2, A3)``, i.e. the endpoint only returns allowances of accounts belonging to `p0`. It is limited to allowances of `p0`, but not only to those having `(p0,s0)` as source of the allowance.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need an example of from_account = (p0, s1) (which I think would just get you the one item because this is in lexicographic order correct?

I still think I like

type ListAllowancesArgs = record {
    from_account : opt { 
        owner = principal;
        subaccount = opt opt blob;
    };
    prev_spender : opt Account;
    take : opt nat;
}

....so that I can indicate I either want All subaccount for a principal with null or A specific subaccount with opt null for default subaccount and opt opt blob for a specific subaccount.

But I seem to recall you may have had a really good reason not to do this? I apologize...the meeting was a while back and I don't quite remember the note.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the API is already not very intuitive so I'd lean towards pushing the implied complexity related to result filtering on the users of the API rather than on the implementation of the API itself. WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants