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

cli_wallet: buy/sell command has incorrect number formatting #544

Closed
HarukaMa opened this issue Dec 19, 2017 · 4 comments
Closed

cli_wallet: buy/sell command has incorrect number formatting #544

HarukaMa opened this issue Dec 19, 2017 · 4 comments

Comments

@HarukaMa
Copy link
Contributor

Example:

unlocked >>> sell haruka BTS CNY 3.3087 10000 false
sell haruka BTS CNY 3.3087 10000 false
10 assert_exception: Assert Exception
rhs.size() <= max_rhs_size:
    {}
    th_a  asset_object.cpp:131 amount_from_string

    {"amount_string":"10000.000000"}
    th_a  asset_object.cpp:146 amount_from_string
unlocked >>>

10000 get converted to 10000.000000, but BTS only has 5 digit precision, thus failing the assert.

@pmconrad
Copy link
Contributor

Thanks for the report.

wallet_api::sell( string seller_account, string base, string quote, double rate, double amount, bool broadcast ) calls sell_asset( seller_account, std::to_string( amount ), base, std::to_string( rate * amount ), quote, 0, false, broadcast )

There are actually two problems here:

  • The sell api (as well as buy) takes a double as parameter which is probably a bad idea
  • It transforms the given numbers into strings using std::to_string(double), which creates a string representation of the number that always has 6 digits. That means you run into the assertion failure with assets that have less than 5 digits, and if you trade assets with more than 6 digits you'll run into rounding errors. For example, a BTC order would always be rounded to the nearest 100 satoshis.

@abitmore
Copy link
Member

@HarukaMa: as @pmconrad said, for some historical reasons, these "new" commands are known to be broken (similar issue reported in #144). Before they're fixed, please use the sell_asset API/command.

marcialvieira added a commit to marcialvieira/bitshares-core that referenced this issue Jan 5, 2018
@abitmore
Copy link
Member

As discussed in #558, let's remove the two commands. Since they're broken, I guess it's safe to do so. Thoughts?

abitmore added a commit to abitmore/bitshares-core that referenced this issue Jan 23, 2018
abitmore added a commit that referenced this issue Jan 23, 2018
Remove buy and sell (with doubles) from CLI. #544
@oxarbitrage
Copy link
Member

closing as commands were removed from the cli.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants