feat(interledger): add sub-accounts with credit and debt #51
Conversation
Add availableCredit and creditExtended account balances
Add totalBorrowed and totalLent account balances
wilsonianb
requested review from
sentientwaffle,
sharafian,
cairin and
matdehaast
Persist new balances even if initial transfer fails.
|
tigerbeetle/tigerbeetle#29 has more context for 950d6b0 👆 |
packages/accounts/migrations/20210422194131_create_ilp_accounts_table.js
Outdated
Show resolved
Hide resolved
Limit use of Postgres type cast to formatDatabaseJson
| @@ -282,68 +257,82 @@ export class AccountsService implements AccountsServiceInterface { | |||
| } | |||
|
|
|||
| public async getAccount(accountId: string): Promise<IlpAccount | undefined> { | |||
| const accountRow = await IlpAccountModel.query().findById(accountId) | |||
| const accountRow = await IlpAccountModel.query() | |||
| .withGraphJoined('subAccounts') | |||
There was a problem hiding this comment.
I think fetching subAccounts will be pretty expensive -- we should avoid doing it unnecessarily (on every getAccount call). Maybe have a different getSubAccounts method?
There was a problem hiding this comment.
@matdehaast do you know what type of call would be most useful for the backend?
- get account optionally with its sub-accounts
- get account optionally with its sub-accounts' ids
- get an account's sub-accounts
- get an account's sub-accounts' ids
There was a problem hiding this comment.
Yeah agreed with @sentientwaffle. An account could potentially have 1000s of subaccounts. We should be very deliberate about getting them. I can't be sure of access patterns yet. Lets just leave the basic for now and add as needed
Define AdjustTrustlineMode enum as strings. Improve mode test readability.
Use withGraphFetched instead of withGraphJoined to avoid DBError: FOR UPDATE cannot be applied to the nullable side of an outer join
Do not query sub-accounts in getAccount
|
What is the 'right' way to create a new trustline without transferring any money to it? |
|
As it is now, it'd be:
If we wanted, we could add a way to create sub-account + extend trustline (steps 1+2) in a single operation. |
|
Yeah, you'd need to call
Every time |
github-actions
bot
added
pkg: interledger
use AccountService.createTransfers for all transfers
wilsonianb
changed the title
Super-accounts that are not top-level may still extend credit to their own sub-accounts.
| if (error) { | ||
| switch (error.code) { |
There was a problem hiding this comment.
Nit: The if { switch { blocks (here and elsewhere in this file) could be simplified to switch (error?.code) { if you add a noop null/undefined case.
There was a problem hiding this comment.
switch (error?.code) doesn't seem to work with createTransfers returning void vs undefined or null
error TS2339: Property 'code' does not exist on type 'void | CreateTransfersError'.
Property 'code' does not exist on type 'void'.
893 switch (error?.code) {
There was a problem hiding this comment.
Huh, that's annoying. Would error && error.code work instead?
There was a problem hiding this comment.
It complains about the subsequent error.indexs and error.code
error TS2339: Property 'index' does not exist on type 'void | CreateTransfersError'.
Property 'index' does not exist on type 'void'.
There was a problem hiding this comment.
Ok, never mind then.
AccountsService.createTransfers doesn't return the error
* feat(accounts): add superAccountId and subAccountIds to IlpAccount * feat(accounts): add extendTrustline Add availableCredit and creditExtended account balances * feat(accounts): add utilizeTrustline Add totalBorrowed and totalLent account balances * feat(accounts): add auto-utilize support to extendTrustline * feat(accounts): add revokeTrustline * feat(accounts): add settleTrustline * fix(accounts): order of fetchByIds result not guaranteed * chore(accounts): clean up adjustTrustline * chore(accounts): consolidate tests * chore(accounts): do not orphan account balances Persist new balances even if initial transfer fails. * chore(accounts): document balance fields * chore(accounts): don't export bigIntToUuid Limit use of Postgres type cast to formatDatabaseJson * chore(accounts): remove obsolete settlement account code * chore(accounts): clean up model data conversion * chore(accounts): document trustline methods Define AdjustTrustlineMode enum as strings. Improve mode test readability. * fix(accounts): lock trustline account rows with forUpdate Use withGraphFetched instead of withGraphJoined to avoid DBError: FOR UPDATE cannot be applied to the nullable side of an outer join * chore(accounts): add getSubAccounts Do not query sub-accounts in getAccount * chore(accounts): create balances on sub-account creation * chore(accounts): refactor trustline methods * chore(interledger): refactor trustline methods use AccountService.createTransfers for all transfers * chore(interledger): replace trustline with credit and debt * feat(interledger): specify creditor super-account Super-accounts that are not top-level may still extend credit to their own sub-accounts. * chore(interledger): move static methods out of AccountsService * chore(interledger): don't require asset to create sub-account * chore(interledger): remove check for linked_event_failed AccountsService.createTransfers doesn't return the error
Changes proposed in this pull request
Context
Resolves #32
Checklist
fixes #number