xchainjs-lib icon indicating copy to clipboard operation
xchainjs-lib copied to clipboard

refactoring Explorers & Providers

Open mikewyszinski opened this issue 3 years ago • 2 comments

mikewyszinski avatar Jul 13 '21 22:07 mikewyszinski

@veado @reidrankin

So i'm partway along in these changes and want to bring up some interface change proposals...

  1. should we remove get/set network?
    • it would simplify the code but force clients to cache the phrase if they want to switch back forth
    • even though i don't like the added state, i'm leaning towards keeping them
  2. modify setPhrase(phrase: string, walletIndex: number): Address -> setPhrase(phrase: string): void
    • this just seems way more intuitive
    • the client will just need to call getAddress() after, if they even need an adress
  3. add the following methods
    • setMasterPublicKey(xpub: string): void - useful for clients to be able to watch wallets without providing a phrase
    • sign(params: TxParams): Promise<SignedTx> - returns a chain specific signed tx
    • broadcastTx(params: SignedTx): Promise<TxHash> - uses providers to broadcast tx
  4. modify getFees(): Promise<Fees> - to return everything
    • currently there are a bunch of fees types, FeeRates, Fees, Fee, FeesWithRates. let's roll them all into one

thats it for now, heres what it looks like together


export interface XChainClient {
  // ===================================
  // Should we ditch these?
  // ===================================
  setNetwork(net: Network): void
  getNetwork(): Network

  // ===================================
  // Change: setPhrase() to not return an address
  // ===================================
  setPhrase(phrase: string): void
  setMasterPublicKey(xpub: string): void

  purgeClient(): void

  // chain specific addres validation
  validateAddress(address: string): boolean

  // ===================================
  // Implemented by HW wallets or in-mem wallet
  // ===================================
  getAddress(walletIndex?: number): Address
  sign(params: TxParams): Promise<SignedTx>

  // ===================================
  // implemented by chain specific Providers
  // ===================================
  getBalance(address: Address, assets?: Asset[]): Promise<Balance[]>
  getTransactions(params?: TxHistoryParams): Promise<TxsPage>
  getTransactionData(txId: string, assetAddress?: Address): Promise<Tx>
  getFees(): Promise<Fees>
  broadcastTx(params: SignedTx): Promise<TxHash>

  // ===================================
  // convenience method
  //  sign() + broadcast() using Providers
  // ===================================
  transfer(params: TxParams): Promise<TxHash>

  // ===================================
  // implemented by chain specific Explorers
  // ===================================
  getExplorerAddressUrl(address: Address): string
  getExplorerTxUrl(txID: string): string
}

mikewyszinski avatar Jul 14 '21 22:07 mikewyszinski

1. should we remove get/set network?

Yes. As you've mentioned it will simplify the code

2. modify `setPhrase(phrase: string, walletIndex: number): Address`   -> `setPhrase(phrase: string): void`

Yes, let's change ReturnType from Address to void

3. add the following methods
   * `setMasterPublicKey(xpub: string): void` - useful for clients  to be able to watch wallets without providing a phrase

What do you mean by watch wallets and why do we need a public key for it?

   * `sign(params: TxParams): Promise<SignedTx>` - returns a chain specific signed tx
   * `broadcastTx(params: SignedTx): Promise<TxHash>` - uses providers to broadcast tx

Yes (for sign / broadcastTx)

4. modify `getFees(): Promise<Fees>` - to return everything
   * currently there are a bunch of fees types, FeeRates, Fees, Fee, FeesWithRates. let's roll them all into one

Not every chain provides or needs FeeRates (e.g. Binance). We could keep values for FeeRates empty for such chain, but what happened if any other (future) chain have another "fee" props we don't care right now. Should we extend Fees then, even it's needed by one (or three) chains? My suggestion is to keep getFees for all chains as it is. And for some "special" cases provide getFeeRates or any other "fee" methods (e.g. getFeeWithRates) - as same as we have already defined it in UTXOClient for xchain-btc|bch|ltc.

veado avatar Jul 15 '21 08:07 veado