Support for native asset & hash in custom Definitions #2597
Conversation
| @@ -0,0 +1,2740 @@ | |||
| { | |||
There was a problem hiding this comment.
This should go in the test folder, not the src folder. And perhaps should be renamed to something like definitions-sample.json so that people don't think it's the current production file.
This is because this file would increase the size of the package by about 6%, and it's therefore not sustainable to do for every XRPL-based chain. I'd recommend maintaining this file in a separate repo and pointing people towards that.
There was a problem hiding this comment.
Will do in separate repo 👍
There was a problem hiding this comment.
The rest of the PR was fine, just this one file was what I suggested you move to a new repo 🙂
There was a problem hiding this comment.
@mvadari Done, loading JSON from external dev-dependency.
| @@ -44,5 +44,8 @@ | |||
| "prettier": "@xrplf/prettier-config", | |||
| "engines": { | |||
| "node": ">= 10" | |||
| }, | |||
| "devDependencies": { | |||
| "xrpl-binary-codec-prerelease": "^7.0.1" | |||
There was a problem hiding this comment.
I would prefer this be a package with just the definitions. The name of it seems like a circular dep. That the stable release would require a pre release to release.
| @@ -1,5 +1,6 @@ | |||
| const { encode, decode, XrplDefinitions } = require('../src') | |||
| const normalDefinitionsJson = require('../src/enums/definitions.json') | |||
| const altDefinitions = require('xrpl-binary-codec-prerelease/dist/enums/definitions-xahau.json') | |||
There was a problem hiding this comment.
I'd recommend having the definitions file be just a copy-pasted test file in the fixtures folder instead of importing a new package, since it doesn't need to be kept up-to-date for the tests to still work.
There was a problem hiding this comment.
You know what: I propose that you decide internally how you want to resolve this :) Lmk if there's anything I can do at this point.
There was a problem hiding this comment.
I agree with @mvadari approach.
High Level Overview of Change
Add native asset & hash as returned by the
server_definitionsmethod toXrplDefinitionsobjects, and allow getting them through:getNativeAsset()getHash()This allows to use these params as present in constructed
XrplDefinitionsby other libs building on top, potentially wanting to passgetNativeAssetto #2596 and usinggetHashto save bandwidth when checking if new definitions are to be loaded.