Skip to content

Update internal data types in reference docs #3164

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

Merged
merged 8 commits into from
Jul 17, 2025
Merged

Conversation

mDuo13
Copy link
Collaborator

@mDuo13 mDuo13 commented Jun 25, 2025

This PR builds on top of #3163 but I split it off because the diff is very large and mostly consists of uninteresting find-and-replace and whitespace changes. (This has been rebased on master after merging that PR.)

  • Consistently use the same names for data types (Fix Inconsistency in "Internal Type" names #3121)
  • Various other formatting / style updates and corrections for various tables that were affected, including:
    • Align of many (not all) tables in MD source
    • Add "Required?" column instead of having (Optional) in the Description column (in many but not all cases—there are a lot)
    • Further clarify the JSON format for some fields, i.e. Hash, Address, hexadecimal fields
    • Remove common fields from transaction-type specific tables.
    • Fix one case where the internal type was specified incorrectly (NFTokenMinter field of AccountSet should be AccountID not "Blob")
    • Fix type definition of Delegate transaction common field (This fixes one of the problems identified in Permission Delegation reference documentation is mixed up #3186.)
    • Replace dark heavy check mark (✔️) emoji with white check mark (✅) emoji for visibility in dark mode on the NFTokenAcceptOffer page.
    • Clarify how tokens can be used in Payment Channels (depends on TokenEscrow amendment) in some references. This fixes part of the problem identified in TokenEscrow functionality isn't tagged with the amendment that adds it #3187.
    • Fix syntax error in JSON sample of MPTokenIssuance and mention XLS-89d on both that page and MPTokenIssuanceCreate.
  • Remove unused frontmatter in a couple files.
  • Improve wording in a few places, like NFTokenAcceptOffer and MPTokenIssuance references.

@@ -27,7 +27,7 @@ _([NonFungibleTokensV1_1 amendment][]により追加されました。)_
## NFTokenID


NFTokenID, 任意, 文字列, Hash256
Copy link
Contributor

Choose a reason for hiding this comment

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

If based on rippled source code, it is Uint256, but if based on server_definition, it is Hash256.

Do you mean to change it to be based on rippled source code?

https://github.com/XRPLF/xrpl.js/blob/0eb77a9458fa515643f18aae2976e5a125b9ed31/packages/ripple-binary-codec/src/enums/definitions.json#L970

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I'm going based on the source code. I do mention the values used in server_definitions.

Copy link
Collaborator

@oeggert oeggert left a comment

Choose a reason for hiding this comment

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

Just a few details need fixing.

Base automatically changed from binary_updates to master July 8, 2025 17:08
- Improve consistency of amendment notices in transaction reference
- Clarify availability of tokens in payment channels
- Other minor fixes
@mDuo13 mDuo13 force-pushed the update_tx_ref_tables branch from 03fc6c1 to e45f5a0 Compare July 8, 2025 18:48
@mDuo13 mDuo13 marked this pull request as ready for review July 8, 2025 22:22
@mDuo13 mDuo13 added the needs japanese translation PRs with English changes that need to be added/updated in the Japanese translation of the site label Jul 8, 2025
@mDuo13 mDuo13 requested a review from oeggert July 8, 2025 22:53
mDuo13 added a commit that referenced this pull request Jul 15, 2025
- Add missing common links snippet.
- Fix case of `Fee` field and string type of example value.
- Update internal type names per #3164
mDuo13 added a commit that referenced this pull request Jul 15, 2025
- Add missing common links snippet.
- Fix case of `Fee` field and string type of example value.
- Update internal type names per #3164
@mDuo13 mDuo13 requested a review from tequdev July 15, 2025 18:58
Copy link
Collaborator

@oeggert oeggert left a comment

Choose a reason for hiding this comment

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

LGTM

@mDuo13 mDuo13 merged commit fe279a1 into master Jul 17, 2025
3 of 4 checks passed
@mDuo13 mDuo13 deleted the update_tx_ref_tables branch July 17, 2025 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs japanese translation PRs with English changes that need to be added/updated in the Japanese translation of the site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency in "Internal Type" names
3 participants