-
Notifications
You must be signed in to change notification settings - Fork 575
feat: add specific response types for ledger_entry request variants #3230
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
8d66a08
69e59c8
9502b35
bdd5d7d
73283c7
e9660a5
e5ca1b9
0419a05
1d0005d
5d49687
005ca74
4395684
44b09e2
e7e2888
0b37135
914075e
a13d291
bd574a9
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 |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import Check from './Check' | |
| import Credential from './Credential' | ||
| import Delegate from './Delegate' | ||
| import DepositPreauth from './DepositPreauth' | ||
| import DID from './DID' | ||
| import DirectoryNode from './DirectoryNode' | ||
| import Escrow from './Escrow' | ||
| import FeeSettings from './FeeSettings' | ||
|
|
@@ -33,6 +34,7 @@ type LedgerEntry = | |
| | Credential | ||
| | Delegate | ||
| | DepositPreauth | ||
| | DID | ||
|
Collaborator
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. Do we need to add MPToken, MPTokenIssuance and NFTokenPage as well? |
||
| | DirectoryNode | ||
| | Escrow | ||
| | FeeSettings | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -230,6 +230,88 @@ export interface LedgerEntryRequest extends BaseRequest, LookupByLedgerRequest { | |
| } | ||
| } | ||
|
|
||
| export interface LedgerEntryAccountRootRequest extends LedgerEntryRequest { | ||
| account_root: Required<LedgerEntryRequest>['account_root'] | ||
| } | ||
|
|
||
| export interface LedgerEntryAMMRequest extends LedgerEntryRequest { | ||
| amm: Required<LedgerEntryRequest>['amm'] | ||
| } | ||
|
|
||
| export interface LedgerEntryBridgeAccountRequest extends LedgerEntryRequest { | ||
| bridge_account: Required<LedgerEntryRequest>['bridge_account'] | ||
| } | ||
|
|
||
| export interface LedgerEntryBridgeRequest extends LedgerEntryRequest { | ||
| bridge: Required<LedgerEntryRequest>['bridge'] | ||
| } | ||
|
|
||
| export interface LedgerEntryCheckRequest extends LedgerEntryRequest { | ||
| check: Required<LedgerEntryRequest>['check'] | ||
| } | ||
|
|
||
| export interface LedgerEntryCredentialRequest extends LedgerEntryRequest { | ||
| credential: Required<LedgerEntryRequest>['credential'] | ||
| } | ||
|
|
||
| export interface LedgerEntryDelegateRequest extends LedgerEntryRequest { | ||
| delegate: Required<LedgerEntryRequest>['delegate'] | ||
| } | ||
|
|
||
| export interface LedgerEntryDepositPreauthRequest extends LedgerEntryRequest { | ||
| deposit_preauth: Required<LedgerEntryRequest>['deposit_preauth'] | ||
| } | ||
|
|
||
| export interface LedgerEntryDIDRequest extends LedgerEntryRequest { | ||
| did: Required<LedgerEntryRequest>['did'] | ||
| } | ||
|
|
||
| export interface LedgerEntryDirectoryRequest extends LedgerEntryRequest { | ||
| directory: Required<LedgerEntryRequest>['directory'] | ||
| } | ||
|
|
||
| export interface LedgerEntryEscrowRequest extends LedgerEntryRequest { | ||
| escrow: Required<LedgerEntryRequest>['escrow'] | ||
| } | ||
|
|
||
| export interface LedgerEntryMPTokenIssuanceRequest extends LedgerEntryRequest { | ||
| mpt_issuance: Required<LedgerEntryRequest>['mpt_issuance'] | ||
| } | ||
|
|
||
| export interface LedgerEntryMPTokenRequest extends LedgerEntryRequest { | ||
| mptoken: Required<LedgerEntryRequest>['mptoken'] | ||
| } | ||
|
|
||
| export interface LedgerEntryNFTokenPageRequest extends LedgerEntryRequest { | ||
| nft_page: Required<LedgerEntryRequest>['nft_page'] | ||
| } | ||
|
|
||
| export interface LedgerEntryOfferRequest extends LedgerEntryRequest { | ||
| offer: Required<LedgerEntryRequest>['offer'] | ||
| } | ||
|
|
||
| export interface LedgerEntryPayChannelRequest extends LedgerEntryRequest { | ||
| payment_channel: Required<LedgerEntryRequest>['payment_channel'] | ||
| } | ||
|
|
||
| export interface LedgerEntryRippleStateRequest extends LedgerEntryRequest { | ||
| ripple_state: Required<LedgerEntryRequest>['ripple_state'] | ||
| } | ||
|
|
||
| export interface LedgerEntryTicketRequest extends LedgerEntryRequest { | ||
| ticket: Required<LedgerEntryRequest>['ticket'] | ||
| } | ||
|
|
||
| // eslint-disable-next-line max-len -- Disable for interface declaration. | ||
| export interface LedgerEntryXChainOwnedClaimIDRequest extends LedgerEntryRequest { | ||
| xchain_owned_claim_id: Required<LedgerEntryRequest>['xchain_owned_claim_id'] | ||
| } | ||
|
|
||
| // eslint-disable-next-line max-len -- Disable for interface declaration. | ||
| export interface LedgerEntryXChainOwnedCreateAccountClaimIDRequest extends LedgerEntryRequest { | ||
| xchain_owned_create_account_claim_id: Required<LedgerEntryRequest>['xchain_owned_create_account_claim_id'] | ||
| } | ||
|
|
||
|
Collaborator
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. Not sure about the scope, but do we need requests for Oracle, Vault, PermissionedDomain, Loan and LoanBroker?
Member
Author
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 not sure if I should add those in this PR, since they didn't exist before.
Collaborator
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. @tequdev Can you log a Github issue detailing missing ledger entires, so that we can close on this refactor PR? |
||
| /** | ||
| * Response expected from a {@link LedgerEntryRequest}. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -232,7 +232,7 @@ describe('PermissionedDEX', function () { | |
| seq: offerCreateTxResponse.result.tx_json.Sequence as number, | ||
| }, | ||
| }) | ||
| ).result.node as Offer | ||
| ).result.node! | ||
|
|
||
| assert.equal(offerLedgerObject.LedgerEntryType, 'Offer') | ||
| assert.equal(offerLedgerObject.DomainID, permDomainLedgerObject.index) | ||
|
|
@@ -259,7 +259,7 @@ describe('PermissionedDEX', function () { | |
| }) | ||
|
|
||
| assert.equal( | ||
| (ledgerEntryResponse.result.node as DirectoryNode).index, | ||
| ledgerEntryResponse.result.node?.index, | ||
|
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. Optional chaining silently passes if Add
Member
Author
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. fixed |
||
| offerLedgerObject.BookDirectory, | ||
| ) | ||
| assert.equal( | ||
|
|
||
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.
What's the scope of this PR? Does it aim to cover all the ledger entries listed on https://xrpl.org/docs/references/protocol/ledger-data/ledger-entry-types?