Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/ripple-binary-codec/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
## Unreleased

### Fixed
- add `MPTAmount` support in `Issue` (rippled internal type)
* add `MPTCurrency` support in `Issue` (rippled internal type)
* Throw an error during serialization if a field is unknown, rather than silently throwing it away.

## 2.3.0 (2025-2-13)

Expand Down
25 changes: 18 additions & 7 deletions packages/ripple-binary-codec/src/types/st-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,25 @@ class STObject extends SerializedType {
return Object.assign(acc, handled ?? { [key]: val })
}, {})

let sorted = Object.keys(xAddressDecoded)
.map((f: string): FieldInstance => definitions.field[f] as FieldInstance)
.filter(
(f: FieldInstance): boolean =>
f !== undefined &&
xAddressDecoded[f.name] !== undefined &&
f.isSerialized,
function isValidFieldInstance(
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This function is needed for typing

f: FieldInstance | undefined,
): f is FieldInstance {
return (
f !== undefined &&
xAddressDecoded[f.name] !== undefined &&
f.isSerialized
)
}

let sorted = Object.keys(xAddressDecoded)
.map((f: string): FieldInstance | undefined => {
if (!(f in definitions.field)) {
if (f[0] === f[0].toLowerCase()) return undefined
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't understand this condition -- Can you point me to the documentation which explains the capitalization of the first character?

I observe examples where the first character of the field-name is both upper and lower case. Ex:

Is there any explanation here? https://xrpl.org/docs/references/protocol/binary-format

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Lower-case fields are synthetic fields that aren't serialized, like hash or date. They're included in tx outputs, so if you're re-feeding that into the binary codec, you want those to be discarded instead of erroring.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

okay thanks. This is useful information, it will be helpful if its mentioned in the docs. Perhaps I missed it?

throw new Error(`Field ${f} is not defined in the definitions`)
}
return definitions.field[f] as FieldInstance
})
.filter(isValidFieldInstance)
.sort((a, b) => {
return a.ordinal - b.ordinal
})
Expand Down
12 changes: 6 additions & 6 deletions packages/ripple-binary-codec/test/definitions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ describe('encode and decode using new types as a parameter', function () {
it('can encode and decode a new Field', function () {
const tx = { ...txJson, NewFieldDefinition: 10 }

// Before updating the types, undefined fields will be ignored on encode
expect(decode(encode(tx))).not.toEqual(tx)
// Before updating the types, undefined fields will throw an error
expect(() => encode(tx)).toThrow()

// Normally this would be generated directly from rippled with something like `server_definitions`.
// Added here to make it easier to see what is actually changing in the definitions.json file.
Expand Down Expand Up @@ -72,8 +72,8 @@ describe('encode and decode using new types as a parameter', function () {
],
}

// Before updating the types, undefined fields will be ignored on encode
expect(decode(encode(tx))).not.toEqual(tx)
// Before updating the types, undefined fields will throw an error
expect(() => encode(tx)).toThrow()

// Normally this would be generated directly from rippled with something like `server_definitions`.
// Added here to make it easier to see what is actually changing in the definitions.json file.
Expand Down Expand Up @@ -141,8 +141,8 @@ describe('encode and decode using new types as a parameter', function () {
},
])

// Test that before updating the types this tx fails to decode correctly. Note that undefined fields are ignored on encode.
expect(decode(encode(tx))).not.toEqual(tx)
// Test that before updating the types this tx fails to decode correctly. Note that undefined fields will throw an error.
expect(() => encode(tx)).toThrow()

class NewType extends UInt32 {
// Should be the same as UInt32
Expand Down
9 changes: 9 additions & 0 deletions packages/ripple-binary-codec/test/tx-encode-decode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,13 @@ describe('encoding and decoding tx_json', function () {
encode(my_tx)
}).toThrow()
})

it('throws when there is an unknown field', function () {
const my_tx = Object.assign({}, tx_json, {
BadField: 1,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If this field was titled badField (beginning with small letter), then the code does not throw. I understand the implementation of the code. But what is rationale for this behavior?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

SFields are always capitalized. Synthetic fields (non-serialized fields that are returned in RPC outputs) are lower-case.

})
expect(() => {
encode(my_tx)
}).toThrow()
})
})
Loading