Change format of get_htlc output

[jmjatlanta]

Fixes #1645

The output of the get_htlc method was cryptic and difficult to parse. This cleans up the formatting of the results.

[jmjatlanta]

The latest version formats the output to what I believe is the intent of the issue. Note that the amount is in "BitShares format", and not decimalized. Is that what is desired?

get_htlc 1.16.17
{
  "transfer": {
    "from": "jmjatlanta1",
    "to": "f0x",
    "asset": "TEST",
    "amount": 10000000
  },
  "conditions": {
    "htlc_lock": {
      "hash_algo": "SHA256",
      "preimage_hash": "8ca187c92a6a3892735ca9fdcc5af91f4f423ee8cda550158192cfe4219246ad",
      "preimage_size": 13
    },
    "time_lock": {
      "expiration": "2019-03-18T15:14:42"
    }
  }
}

[ryanRfox]

Yes, User is required to know the asset precision. Consistent with other responses.

[jmjatlanta]

The latest commits change the internal representation to somewhat match the output of the cli_wallet::get_htlc() call. Unfortunately, there must exist ugly code in order to "pretty" the eventual output for the user. I guess the ugly code is a necessary evil, but if some have suggestions to improve it, I would be happy to consider them.

[abitmore]

I guess it's better to always format amount as a string, btw pay attention to precision, see

bitshares-core/libraries/app/database_api.cpp

Lines 418 to 419 in f087fe8

	base_volume = uint128_amount_to_string( bv, asset_base.precision );
	quote_volume = uint128_amount_to_string( qv, asset_quote.precision );

.

[jmjatlanta]

I guess it's better to always format amount as a string, btw pay attention to precision

So by that comment you think we should include the decimal? Or are you only saying the amount should be enclosed in quotes to be read as a string?

[abitmore]

For readability, IMHO we should include the decimal (I know get_objects returns pure raw data).

[jmjatlanta]

Should we round to a certain decimal place, or just print the results? Here is the result of simply casting to double and dividing by the precision of the asset:

get_htlc 1.16.18
{
  "transfer": {
    "from": "jmjatlanta1",
    "to": "f0x",
    "asset": "TEST",
    "amount": "2000000.00000000000000000"
  },
  "conditions": {
    "htlc_lock": {
      "hash_algo": "SHA256",
      "preimage_hash": "5899575803417e3356a133c51efff8314c0d3d7a52f37472f90b1dce5288525b",
      "preimage_size": 13
    },
    "time_lock": {
      "expiration": "2019-03-19T13:26:00"
    }
  }
}

[jmjatlanta]

How embarrassing. The answer is there in my previous comment. We have the precision. I will cut the zeros off at the appropriate place.

[abitmore]

Don't round in a serious financial system. Don't use double. Why are there so many zeros? If TEST's precision is 5, there should be only 5 digits after the dot. I guess you missed the link above.

bitshares-core/libraries/app/util.cpp

Line 54 in f087fe8

string uint128_amount_to_string( const fc::uint128& amount, const uint8_t precision )

Update: just saw the latest comment/commit.

[ryanRfox]

If appropriate to add a change order at this late moment, please consider adding "time_lock countdown" or similar:

    "time_lock": {
      "expiration": "2019-03-19T13:26:00"
      "countdown": "5 minutes in the future"

[oxarbitrage]

I am not sure if this will work, as far as i know object.valid() is used to check optionals in bitshares codebase. If we change to valid() then if (!obj.is_null()) in line 687 can be changed for consistency as well.

[jmjatlanta]

I fixed this location to use the more expressive .valid(). The object on line 687 is not an optional, but a variant. So I left it as .is_null()

[oxarbitrage]

It looks good to me @jmjatlanta . From learned lessons in the get_ticker i think we could test the output. When get_htlc is called in cli_create_htlc we can hardcode and check the object keys.

This way if we some day change the output in an incompatible manner for any reason test will fail.

This, if useful can be done in a separated issue.

[jmjatlanta]

This, if useful can be done in a separated issue.

I believe that would be good to do. But as not to hold up progress, I would ask that it be done in a separate issue, which I will create (and probably do the work on).

[oxarbitrage]

thank you @jmjatlanta