search

HathorNetwork / hathor-wallet-headless

Headless Hathor Wallet
https://hathor.network/
MIT License
16 stars 23 forks source link

[Goal #3] Design get address info #56

Closed thr0wn closed 3 years ago

thr0wn commented 3 years ago

Motivation

An API to get information about a specific address is helpful for a variety of use cases. Some of them are:

Solution

hathor-wallet-lib

HathorWallet is declared at src/new/wallet.js

A new method called getAddressInfo must be created on HathorWallet to return the address information. It is important to note that to calculate total_amount_received, total_amount_sent, total_amount_available and total_amount_locked will be necessary to iterate through the transaction history.

HathorWallet.getAddressInfo interface:

interface getAddressInfo {
    // address: Address to get information of (Required: true) 
    // options.token: Token used to calculate the received and the sent amount (Required: false. Default: HTR) 
    (address: string, options: { token: string }): GetAddressInfoReturn;
}

interface GetAddressInfoReturn {
    // Sum of all amounts received 
    "total_amount_received": number,
    // Sum of all amounts sent 
    "total_amount_sent": number,
    // Amount available to transfer
    "total_amount_available": number,
    // Amount locked, not available to transfer
    "total_amount_locked": number,
    // Token used to calculate the received and the sent amounts
    "token": string,
    // Derivation path index
    "index": number
}

HathorWallet.getAddressInfo should throw an error to the following cases:

GET /wallet/address-info (hathor-wallet-headless)

Endpoint

/wallet/address-info

Query Parameters Parameter Description Type Required Default
address Address to get information of string true
token Token used to calculate the received and the sent amounts string false HTR

Response Success: Same as GetAddressInfoReturn above.

Error when the address does not belong to hathor-wallet-headless:

    success: false,
    error: "Address does not belong to this wallet."

Error when the address is invalid:

    success: false,
    error: "Invalid address."

Tasks

Estimated time

3 days

pedroferreira1 commented 3 years ago

I like the design, just some comments and suggestions:

  1. I think we should be able to filter by token also. Then we would add another parameter in the GET API (token) for the token UID and in the lib as well.
  2. We could follow what we've been using in the HathorWallet new methods of having an options parameter for the optional ones. Then the method signature would be getAddressInfo(address, options = {}), where options could have include_utxos and token.
  3. Can you describe what are the return fields script_pub_key and is_script and their usage?
  4. You've added possible return errors in the headless, which is great but I think you could explicitly describe the validations in the lib method that would generate those errors. Just say that we must validate that the address is valid and it belongs to the loaded wallet.
  5. It still need the task estimation.

What do you think?

thr0wn commented 3 years ago

Thanks for your suggestions.

  1. Done
  2. Done.
  3. Added some comments regarding script_pub_key and is_script, but I am not 100% sure of their usage. I've only documented it here because yiimp has references to those fields. But I will investigate more and post any updates on it.
  4. Done.
  5. Done.
pedroferreira1 commented 3 years ago

For me it's approved.

  1. I think we should explicitly write the fields that are required in the GET request query params and the default values for the ones that are not required.
  2. I am not sure about the fields we are adding because of yiimp. If we don't know for sure their usage maybe we should start without them and add them later if needed. What do you think? @thr0wn @msbrogli
thr0wn commented 3 years ago

@pedroferreira1

  1. Done.
  2. Agreed. Properties removed. To keep a registry of it, the properties removed were: 
    {
// Base64 encoded locking script (Currently only P2PKH or P2SH)
// Same script as the outputs received by this address or empty ""
"script_pub_key": string,
// True if the script_pub_key is not empty
"is_script": boolean,
// pubkeyhash or scripthash, otherwise unknown
"script_type" : string,
}

Note that script_pub_key will be identical to the scripts of its funds (received outputs).

msbrogli commented 3 years ago

@thr0wn Here are my comments:

1) GetAddressInfoReturn should include which token it is related to.

2) GetAddressInfoReturn should limit the number of utxos to a maximum (or maybe just never include it and let the UTXO Consolidation API handle the utxos).

3) GetAddressInfoReturn should not send the pubKey because it is a sensitive information. We might allow the user to request the pubKey but I'm not sure about it. Why would we do it?

thr0wn commented 3 years ago

Thanks for your comments @msbrogli.

  1. Done.
  2. I agree with utxo consolidation api handling utxos. Added a note about it.
  3. Done. PubKey removed.
msbrogli commented 3 years ago

@thr0wn What note? I feel we should just remove the utxos at all. They are not useful here, aren't they?

thr0wn commented 3 years ago

@msbrogli, I misunderstood point 2, sorry. utxos removed.

msbrogli commented 3 years ago

Approved.