xrpl.js
xrpl.js copied to clipboard
add ExpandedSignerList amendment support
High Level Overview of Change
Adds ExpandedSignerList amendment support.
Context of Change
- Expands the maximum signer list size to 32 entries.
- Adds an optional field
WalletLocator
toSignerEntry
. - rippled PR
Type of Change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Refactor (non-breaking change that only restructures code)
- [ ] Tests (You added tests for code that already exists, or your new feature included in this PR)
- [ ] Documentation Updates
- [ ] Release
Test Plan
Adds unit & integration tests.
Future Tasks
Merge this PR once the amendment goes live.
It is possible to rename the field to SignerMetadata
since the actual data is in a binary format, and clients can interpret (map) / encode any field names they want. But given the code in rippled and on the docs, keeping the WalletLocator
name is certainly the path of least resistance
It is possible to rename the field to
SignerMetadata
since the actual data is in a binary format, and clients can interpret (map) / encode any field names they want. But given the code in rippled and on the docs, keeping theWalletLocator
name is certainly the path of least resistance
@intelliot I believe the better path is to use the same parameter name, WalletLocator
, that's on the docs to avoid confusion. AFAIK, we've been consistent with our models containing the same field names as what's on the docs. If users do get confused by the name, they can read the JSDocs or the official docs to understand its usage. In addition, we can propose the name change SignerMetadata
to rippled team and see if they can refactor the name and publish the change. Not sure if this would have to be in a different amendment or if they can modify the amendment?
@intelliot I believe the better path is to use the same parameter name,
WalletLocator
, that's on the docs to avoid confusion. AFAIK, we've been consistent with our models containing the same field names as what's on the docs. If users do get confused by the name, they can read the JSDocs or the official docs to understand its usage.
Re: the first part, just wanted to add +1 to staying consistent with rippled's naming/interface.
The rippled team is aware of this and there's no need to change it. It was discussed in the PR you linked to, https://github.com/ripple/rippled/pull/4097
There's nothing strictly canonical about the JSON format: only the binary really matters at a protocol level. But given that rippled is currently the only implementation, I agree that we should just use the same names unless there is a very good reason not to.
- Can we add a caveat in the release notes to say that the limit is 8 unless the ExpandedSignerList amendment is active?
- What error should users expect if they attempt to set 9+ signers on a network where
ExpandedSignerList
is not active? It may be useful to document this in the release notes (or elsewhere) as well.