Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataAPI for address validation #72

Open
LanfordCai opened this issue Dec 10, 2020 · 3 comments
Open

DataAPI for address validation #72

LanfordCai opened this issue Dec 10, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@LanfordCai
Copy link

LanfordCai commented Dec 10, 2020

I'm working for an exchange and have integrated a blockchain by using Rosetta. This project is really amazing and is well compatible with our current blockchain integration process. But there is a feature I want to request.

Is your feature request related to a problem? Please describe.

We will check the address that the customer input in the withdrawal process:

  1. customer might input an invalid address by mistake, in which case, we should remind him/her to do re-check. For example, 1JTxUeSwH5Ba2ib2UD6Whv5ZXB8RRjfRM8 is a valid address but 1JTxUeSwH5Ba2ib2UD6Whv5ZXB8RRjfRM9 is not.
  2. customer might input an address in the format which we don't support yet or are not ready to open to customers. For example, for Bitcoin, we support the withdrawal to P2PKH and P2SH address , but we don't support the withdrawal to P2WPKH and P2WSH address.

For the blockchain we have integrated via Rosetta, I used /account/balance to check if the address is valid or not; For invalid address, it would returninvalid account name. But I think it's just a workaround, and is not working for the case 2 above.

Describe the solution you'd like

I am wondering if Rosetta can add one more API, like /account/info:

  1. for case 1, it returns "invalid account"
  2. for case 2, it returns something like this:
{
 "account_identifier": {
  "address": "1JTxUeSwH5Ba2ib2UD6Whv5ZXB8RRjfRM8",
  "sub_account": {
   "address": "aabbcc",
   "metadata": {}
  },
  "metadata": {},
  "type": "P2PKH"
 }
}

The type field is not in the metadata for it's not optional.

@LanfordCai LanfordCai added the enhancement New feature or request label Dec 10, 2020
@patrick-ogrady
Copy link
Contributor

Thanks for posting about this, @LanfordCai!

customer might input an invalid address by mistake, in which case, we should remind him/her to do re-check. For example, 1JTxUeSwH5Ba2ib2UD6Whv5ZXB8RRjfRM8 is a valid address but 1JTxUeSwH5Ba2ib2UD6Whv5ZXB8RRjfRM9 is not.

We perform this sort of logic check prior to invoking Rosetta but I see the appeal in having it integrated directly into the implementation. I think the responsibility to do this sort of rejection should fall on the implementation. Maybe the best way to enforce this would be to add some testing to rosetta-cli to purposely create txs with invalid addresses?

customer might input an address in the format which we don't support yet or are not ready to open to customers. For example, for Bitcoin, we support the withdrawal to P2PKH and P2SH address , but we don't support the withdrawal to P2WPKH and P2WSH address.

We've been thinking about this problem a lot lately (multiple address formats). Right now, Rosetta doesn't really have a notion of different address types and it is creating some confusion for folks trying to integrate with rosetta-bitcoin using non Segwit-Bech32 address formats (i.e. how do you specify which address type to create in /construction/derive). A somewhat related question folks have is how to get all available SubAccountIdentifiers for a particular `AccountIdentifier.

I am wondering if Rosetta can add one more API, like /account/info:

Do you envision this being offline or online? I could see an appeal in also returning "nonce-like" info in this sort of response.

@LanfordCai
Copy link
Author

LanfordCai commented Dec 11, 2020

Hi, thank you for your response, @patrick-ogrady !

We perform this sort of logic check prior to invoking Rosetta but I see the appeal in having it integrated directly into the implementation. I think the responsibility to do this sort of rejection should fall on the implementation. Maybe the best way to enforce this would be to add some testing to rosetta-cli to purposely create txs with invalid addresses?

In our exchange, basically three steps are involved in the withdrawal process:

  1. validate the address
  2. create and sign transaction(by calling Rosetta API)
  3. broadcast transaction(by calling Rosetta API)

It would be better if step 1 can also be done by calling a specific Rosetta API. I think with Rosetta, we don't need to know lots of technical details about the blockchain, including how to validate an address(which requires knowledge about the address generation process). It is indeed acceptable to return ‘error’ when creating a transaction with invalid address, but it would be better if we can reject the invalidate address before creating the transaction.

We've been thinking about this problem a lot lately (multiple address formats). Right now, Rosetta doesn't really have a notion of different address types and it is creating some confusion for folks trying to integrate with rosetta-bitcoin using non Segwit-Bech32 address formats (i.e. how do you specify which address type to create in /construction/derive). A somewhat related question folks have is how to get all available SubAccountIdentifiers for a particular `AccountIdentifier.

It would be great if we can specify address types when deriving an address and getting the type information in AccountIdentifier of an address.

Do you envision this being offline or online? I could see an appeal in also returning "nonce-like" info in this sort of response.

For format validation, offline is enough. We can add type field or format field to AccountIdentifier. Unlike nonce, the format of an address is unchangeable, so it's improper to put it into metadata I think.

But if we take a further step, some information about an address is only available in online mode. Ethereum addresses are checksumed hex, but it might be an EOA or smart contract, similarly, addresses of EOS like blockchains might be an registered account without/with code deployed. Those info can only be accessed by querying the blockchain and all of these information can be put into metadata field, depending on the implementation. The AccountIdentifier may look like these:

EOS Premium Account:

{
  "account_identifier": {
    "address": "b1",
    "type": "Premium"
    "metadata": {
      "registered": true
    }
  }
}

EOS Normal Account:

{
  "account_identifier": {
    "address": "huobideposit",
    "type": "Normal"
    "metadata": {
      "registered": true
    }
  }
}

BCH Address:

{
  "account_identifier": {
    "address": "1JTxUeSwH5Ba2ib2UD6Whv5ZXB8RRjfRM8",
    "type": "P2PKH",
    "metadata" {
      "cash_addr": "bitcoincash:qzlexs6dcr4qpc7u5pn36pz8luavhx2qjqy4pny7gp"
    }
  }
}

@patrick-ogrady
Copy link
Contributor

Thanks for providing this additional context, @LanfordCai. I'd recommend you chime in on a thread about a different use for /account/info. I think we could probably merge these workstreams:

https://community.rosetta-api.org/t/representing-minas-vesting-accounts-using-balance-exemptions/317/7?u=patrick.ogrady

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants