XRPLF / xrpl.js

A JavaScript/TypeScript API for interacting with the XRP Ledger in Node.js and the browser
https://xrpl.org/
1.2k stars 510 forks source link

AccountSet flags by name #1483

Closed mDuo13 closed 2 years ago

mDuo13 commented 3 years ago

AccountSet transaction flags (tf) have aliases in txFlags.AccountSet but [AccountSet flags (asf)](https://xrpl.org/accountset.html#accountset-flags) don't have aliases in the API. So for example, preparing an AccountSet using prepareTransaction() currently has to look kind of like t his:

const accountset_tx_json = {
    "Account": cold_address,
    "TransferFee": 0,
    "TickSize": 5,
    "SetFlag": 8, // enable Default Ripple
    "Flags": (api.txFlags.AccountSet.DisallowXRP | 
              api.txFlags.AccountSet.RequireDestTag)
  }

It would be nice if there were some constants that could be passed to SetFlag here. For example, something like this:

const accountset_tx_json = {
    "Account": cold_address,
    "TransferFee": 0,
    "TickSize": 5,
    "SetFlag": api.accountsetFlags.DefaultRipple,
    "Flags": (api.txFlags.AccountSet.DisallowXRP | 
              api.txFlags.AccountSet.RequireDestTag)
  }

(Note, this example uses a mix of transaction Flags and AccountSet flags because that makes it possible to enable 3 flags in one transaction.)

intelliot commented 3 years ago

@mDuo13 I think the aliases are here: https://github.com/ripple/ripple-lib/blob/develop/src/common/txflags.ts#L44-L59

I'm not sure if they are exported cleanly or documented well, though. Worth looking into.

JST5000 commented 3 years ago

From poking around a bit it seems the export works fine, and the documentation seems to match that of txFlags, so I'm going to close this. (If there's a more concrete improvement I'm missing, happy to reopen in that case)

mDuo13 commented 3 years ago

I can't find where they're exported or documented.

CTRL-F on https://xrpl.org/rippleapi-reference.html for "asfDefaultRipple" gives a not-found result.

And they don't seem to be in the API object as far as I can tell. (api.txFlags.AccountSet is there, but that contains the tf flags, not the asf flags.)

mDuo13 commented 3 years ago

Confirmed that it's now possible (as of 1.10.0) to reference the flags by name under ripple.RippleAPI.accountSetFlags. However, it's still not possible to reference the flags on an actual instance of the RippleAPI class. It's confusing that the way you access these names is different than the way you access txFlags:

const ripple = require('ripple-lib')
api = new ripple.RippleAPI({server: 'wss://s.altnet.rippletest.net:51233'})

console.log(api.txFlags)
// Object { Universal: {…}, AccountSet: {…}, TrustSet: {…}, OfferCreate: {…}, Payment: {…}, PaymentChannelClaim: {…} }
console.log(api.accountSetFlags)
// undefined

console.log(ripple.RippleAPI.accountSetFlags)
// Object { requireDestinationTag: 1, requireAuthorization: 2, depositAuth: 9, disallowIncomingXRP: 3, disableMasterKey: 4, enableTransactionIDTracking: 5, noFreeze: 6, globalFreeze: 7, defaultRipple: 8 }
console.log(ripple.RippleAPI.txFlags)
// undefined

Please make it so that txFlags and accountSetFlags can be accessed in the same way. Preferably both types of flags could be accessed both ways. At least make it so both can be accessed the from an API instance object.

EDIT: also, the documentation looks like it's not formatting correctly on GitHub nor on xrpl.org.

intelliot commented 3 years ago

For ripple-lib v1.x, we should allow the flags to be accessed both ways.

In xrpl.js v2.0, I believe the flags will only be accessible statically (not on an instance of a class), since they do not depend on having a Client instance.