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

fix: resolve TransactionStream and Ledger model issues #2779

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions packages/xrpl/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
### Added
* parseTransactionFlags as a utility function in the xrpl package to streamline transactions flags-to-map conversion

### Fixed
* `TransactionStream` model supports APIv2
* `TransactionStream` model includes `close_time_iso` field
* `Ledger` model includes `close_time_iso` field

Comment on lines +10 to +14
Copy link

@coderabbitai coderabbitai bot Oct 8, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Version 1.2.5 introduces important bug fixes and changes

This version addresses a critical issue related to transaction signing:

  • Fixed a bug that could cause transactions to be serialized incorrectly during the signing process.
  • This fix is particularly important for amounts of XRP with more than 6 decimal places and amounts of XRP drops with any decimal places.
  • Users should update to this version to ensure accurate transaction signing and avoid potential issues with transaction processing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't version 1.2.5.

Copy link

Choose a reason for hiding this comment

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

@mvadari, thank you for pointing that out. I apologize for the oversight. I will make sure to reference the correct version in my future reviews.


✏️ Learnings added
Learnt from: mvadari
PR: XRPLF/xrpl.js#2779
File: packages/xrpl/HISTORY.md:10-14
Timestamp: 2024-10-11T18:48:28.370Z
Learning: Ensure to reference the correct version numbers when commenting on changes to `HISTORY.md` to avoid confusion.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

## 4.0.0 (2024-07-15)

### BREAKING CHANGES
Expand Down
5 changes: 3 additions & 2 deletions packages/xrpl/src/client/partialPayment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { decode } from 'ripple-binary-codec'
import type {
TransactionEntryResponse,
TransactionStream,
TransactionV1Stream,
TxResponse,
} from '..'
import type { Amount, APIVersion, DEFAULT_API_VERSION } from '../models/common'
Expand Down Expand Up @@ -159,10 +160,10 @@ export function handlePartialPayment<
* @param log - The method used for logging by the connection (to report the partial payment).
*/
export function handleStreamPartialPayment(
stream: TransactionStream,
stream: TransactionStream | TransactionV1Stream,
log: (id: string, message: string) => void,
): void {
if (isPartialPayment(stream.transaction, stream.meta)) {
if (isPartialPayment(stream.transaction ?? stream.tx_json, stream.meta)) {
mvadari marked this conversation as resolved.
Show resolved Hide resolved
const warnings = stream.warnings ?? []

const warning = {
Expand Down
5 changes: 5 additions & 0 deletions packages/xrpl/src/models/ledger/Ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ interface BaseLedger {
* by which the close_time could be rounded.
*/
close_time_resolution: number
/**
* The approximate time this ledger was closed, in date time string format.
* Always uses the UTC time zone.
*/
close_time_iso: string
/** Whether or not this ledger has been closed. */
closed: boolean
/**
Expand Down
2 changes: 2 additions & 0 deletions packages/xrpl/src/models/methods/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ import {
SubscribeRequest,
SubscribeResponse,
TransactionStream,
TransactionV1Stream,
ValidationStream,
} from './subscribe'
import {
Expand Down Expand Up @@ -583,6 +584,7 @@ export {
LedgerStreamResponse,
ValidationStream,
TransactionStream,
TransactionV1Stream,
PathFindStream,
PeerStatusStream,
OrderBookStream,
Expand Down
37 changes: 34 additions & 3 deletions packages/xrpl/src/models/methods/subscribe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import type {
Path,
StreamType,
ResponseOnlyTxInfo,
APIVersion,
DEFAULT_API_VERSION,
RIPPLED_API_V1,
RIPPLED_API_V2,
} from '../common'
import { Offer } from '../ledger'
import { OfferCreate, Transaction } from '../transactions'
Expand Down Expand Up @@ -262,9 +266,16 @@ export interface ValidationStream extends BaseStream {
*
* @category Streams
*/
export interface TransactionStream extends BaseStream {
interface TransactionStreamBase<
Version extends APIVersion = typeof DEFAULT_API_VERSION,
> extends BaseStream {
status: string
type: 'transaction'
/**
* The approximate time this ledger was closed, in date time string format.
* Always uses the UTC time zone.
*/
close_time_iso: string
/** String Transaction result code. */
engine_result: string
/** Numeric transaction response code, if applicable. */
Expand All @@ -285,8 +296,14 @@ export interface TransactionStream extends BaseStream {
* in detail.
*/
meta?: TransactionMetadata
/** The definition of the transaction in JSON format. */
transaction: Transaction & ResponseOnlyTxInfo
/** JSON object defining the transaction. */
tx_json?: Version extends typeof RIPPLED_API_V2
? Transaction & ResponseOnlyTxInfo
: never
/** JSON object defining the transaction in rippled API v1. */
transaction?: Version extends typeof RIPPLED_API_V1
? Transaction & ResponseOnlyTxInfo
: never
/**
* If true, this transaction is included in a validated ledger and its
* outcome is final. Responses from the transaction stream should always be
Expand All @@ -296,6 +313,20 @@ export interface TransactionStream extends BaseStream {
warnings?: Array<{ id: number; message: string }>
}

/**
* Expected response from an {@link AccountTxRequest}.
*
* @category Streams
*/
export type TransactionStream = TransactionStreamBase

/**
* Expected response from an {@link AccountTxRequest} with `api_version` set to 1.
*
* @category Streams
*/
export type TransactionV1Stream = TransactionStreamBase<typeof RIPPLED_API_V1>

/**
* The admin-only `peer_status` stream reports a large amount of information on
* the activities of other rippled servers to which this server is connected, in
Expand Down
1 change: 1 addition & 0 deletions packages/xrpl/test/fixtures/requests/hashLedger.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"close_flags": 0,
"ledger_index": "15202439",
"close_time_human": "2015-Aug-12 01:01:10.000000000 UTC",
"close_time_iso": "2015-08-12T01:01.10Z",
"close_time_resolution": 10,
"closed": true,
"hash": "F4D865D83EB88C1A1911B9E90641919A1314F36E1B099F8E95FE3B7C77BE3349",
Expand Down
Loading