wallet-only mode

[shamardy]

fixes #864

[artemii235]

@shamardy Thanks for the PR!
Could you please check the proposed minor changes? 🙂

[artemii235]

LGTM, thanks!
@tonymorony Could you please test the PR using the checklist? #864 (comment)

[SirSevenG]

@artemii235
Checklist:

Tested on "result": "2985c7cb6" linux build.

1. Ensure that orderbook RPC returns an error if the base and/or rel are wallet only.

passed

2. best_orders should not accept the wallet-only coin as param.

passed

3. best_orders should filter out wallet-only coins orders from the response.

Returns error "error": "rpc:253] best_orders:130] Coin WONLY is wallet only" if there is any wallet-only coins in response instead of filtering.

4. Recheck that buy/sell/setprice returns an error if the base and/or rel are wallet only.

passed

5. trade_preimage should return an error if the base and/or rel are wallet only.

passed

6. orderbook_depth should return an error if at least 1 pair contains a wallet-only coin.

passed

On the side note: outside of "wallet_only" option for coins config, should we default to wallet_only every coin with "mm2":0 in coins file and/or electrum/enable call?

[SirSevenG]

See conversation.

[artemii235]

@shamardy Could you please fix the

3. best_orders should filter out wallet-only coins orders from the response.
   Returns error "error": "rpc:253] best_orders:130] Coin WONLY is wallet only" if there is any wallet-only coins in response instead of filtering.

Other nodes might have different configs and treat the WONLY as available for trade. This should not cause RPC errors on the node considering WONLY as wallet-only. It might be even a vector for DoS in such a case.

[artemii235]

@SirSevenG

On the side note: outside of "wallet_only" option for coins config, should we default to wallet_only every coin with "mm2":0 in coins file and/or electrum/enable call?

"mm2":0 won't allow enabling the coin at all. Some of such coins can be turned to wallet only + "mm2":1, but we should ensure that they work in this mode correctly.

[SirSevenG]

@artemii235

"mm2":0 won't allow enabling the coin at all. Some of such coins can be turned to wallet only + "mm2":1, but we should ensure that they work in this mode correctly.

On the same build, I was able to enable coin with "mm2": 0, also crate orders with this coin. Should we create a separate issue regarding?

Example:

(coins file entry - MORTY clone)

{
                "coin": "tMORTY",
                "asset": "tMORTY",
                "fname": "tMORTY (TESTCOIN)",
                "rpcport": 16348,
                "txversion": 4,
                "wallet_only": true,
                "overwintered": 1,
                "mm2": 0,
                "required_confirmations": 1,
                "requires_notarization": false,
                "avg_blocktime": 1,
                "protocol": {
                        "type": "UTXO"
                }
        },

curl --url "http://127.0.0.1:7783" --data "{\"userpass\":\"test\",\"method\":\"electrum\",\"mm2\":0,\"coin\":\"tMORTY\",\"servers\":[{\"url\":\"electrum3.cipig.net:10018\",\"protocol\":\"TCP\",\"disable_cert_verification\":true},{\"url\":\"electrum1.cipig.net:10018\"}]}"

{
  "result": "success",
  "address": "RADDRESS",
  "balance": "7.777",
  "unspendable_balance": "0",
  "coin": "tMORTY",
  "required_confirmations": 1,
  "requires_notarization": false,
  "mature_confirmations": 100
}

UPD: opened issue #919

[artemii235]

On the same build, I was able to enable coin with "mm2": 0, also crate orders with this coin. Should we create a separate issue regarding?

Yes, please, as it's irrelevant to this PR. Please share the config of the coin, the request body, etc. in the issue description.

[artemii235]

@SirSevenG Please retest the latest commit.

[SirSevenG]

Returns error "error": "rpc:253] best_orders:130] Coin WONLY is wallet only" if there is any wallet-only coins in response instead of filtering.

Returns filtered orders json, as intended.
mm2 stodout prints best_orders:130] WARN Coin WONLY was removed from best orders because it's defined as wallet only in config

Found no new issues.

[SirSevenG]

[artemii235]

@SirSevenG Thanks for testing!

[artemii235]

🔥