XRPLF / xrpl.js

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

make the Wallet more extensible. #2602

Open drunkirishcoder opened 11 months ago

drunkirishcoder commented 11 months ago

I would like to implement a wallet that signs remotely and be able to integrate with xrpl. something like

const wallet = new MyRemoteWallet(...)

const res = await client.submitAndWait({
    TransactionType: 'Payment',
    Account: wallet.address,
    Destination: "r...",
    Amount: '1',
  },
  {
    autofill: true,
    wallet,
  })

I can extend my remote wallet from the xrpl wallet, export class MyRemoteWallet extends xrpl.Wallet, but the problem is the sign function is not async, therefore I can't make a remote call from it. though because submitAndWait is async, it should be able to support an async sign function.

essentially if this block of code can be changed to

  let tx =
    typeof transaction === 'string'
      ? // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- converts JsonObject to correct Transaction type
        (decode(transaction) as unknown as Transaction)
      : transaction

  if (autofill) {
    tx = await client.autofill(tx)
  }

  return (await wallet.sign(tx)).tx_blob

everything would work. this change would make extending xrpl with remote wallets much easier.

tequdev commented 2 months ago

We should seriously discuss about this.

And, it is not enough to simply make Wallet.sign() an async function, as we sometimes want to sent transactions on the remote wallet side (not xrpl.js side) when signing with Remote Wallet within client.signAndSubmit().

tequdev commented 2 months ago

And it is also necessary to match the Client's connection network with the RemoteWallet's connection network. We need to consider whether we simply pass the NetworkID or use a different identifier.

This Standard proposal would greatly help this Issue. https://github.com/XRPLF/XRPL-Standards/discussions/206