Incompatibility between Core and Unlimited

[Steve Sokolowski]

For the past several weeks, we've had complaints from customers about Coinbase payouts not being sold. This stumped us for a while until we discovered an exception message that was recorded to our database periodically - "incorrect amount."

It turns out that Bitcoin Unlimited requires that while our system tracks balances to 24 decimal places, input to the "sendmany" API function must be truncated to 8 decimal places. For example, we may instruct Bitcoin Unlimited to send 0.004342387789045237582390 bitcoins to an address, but it will only accept an amount of 0.00434238. Otherwise, it fails and does not send the money by returning "incorrect amount."

This is not the behavior of Bitcoin Core and other coins forked from bitcoin that use its JSON RPC API specification. Those coins will accept an unlimited number of decimal places and truncate automatically. Due to this problem, we reverted to the Core until a fix is available.

Neither of these behaviors is "correct," but since the Core and the other coins accept any length of decimal places, it seems that Bitcoin Unlimited should conform to the traditional specification to avoid compatibility issues.

I don't have the time to write and test the fix, so hopefully this report will reach one of the developers of Bitcoin Unlimited, who can make the software execute the same behavior used by the other coins.

 

[freetrader]

Steve, thanks for bringing this to attention. I've notified the developers on the Slack channel, though I think that due to timezones it may take a little while to get a response.

Perhaps you could indicate the precise software versions in your comparison, to help with the analysis.

 

[Peter Tschipper]

@Steve Sokolowski Steve, I agree with freetrader, we'll need to know which version of BU you're using and the exact error string from the logfile would help also...thanks.

 

[freetrader]

@Steve Sokolowski :

I took a closer look at the code in question (good chance for me to learn )

What I found seems to indicate that this is a result of a difference between Core 0.11.2 and Core 0.12, rather than a BU/Core incompatibility.

Firstly, I looked at the code for Core 0.11.2 and the 0.12 versions of Core and BU (actually, Classic too, but nevermind that).

The following Core 0.12 commit revamped how JSON amount strings are parsed:
https://github.com/bitcoin/bitcoin/commit/9cc91523dbec6441e327e1e4c83ba751a4680bec

Differences between 0.12 Core and 0.12 BU in the files changed by this commit are negligible (don't affect the decision in this question), so 0.12 BU should behave exactly like Core 0.12 in this respect. This also matches the expectations of the developers.

BTW, I suspect the exact error message you got is "Invalid amount", not "incorrect amount", because the latter string does not occur in the source (neither 0.11.2 nor 0.12 clients).

To check I introduced a test in Core 0.12 util_tests.cpp, which contains the unit tests for the new ParseFixedPoint function. I used the value you gave in the thread post:

diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 28cecff..a4ef569 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -455,6 +455,9 @@ BOOST_AUTO_TEST_CASE(test_ParseFixedPoint)
BOOST_CHECK(ParseFixedPoint("-9999999999.99999999", 8, &amount));
BOOST_CHECK_EQUAL(amount, -999999999999999999LL);
+ BOOST_CHECK(ParseFixedPoint("0.004342387789045237582390", 8, &amount));
+ BOOST_CHECK_EQUAL(amount, 434238LL);
+
BOOST_CHECK(!ParseFixedPoint("", 8, &amount));
BOOST_CHECK(!ParseFixedPoint("-", 8, &amount));
BOOST_CHECK(!ParseFixedPoint("a-1000", 8, &amount));

When run, the test fails:

test/util_tests.cpp(458): error in "test_ParseFixedPoint": check ParseFixedPoint("0.004342387789045237582390", 8, &amount) failed
test/util_tests.cpp(459): error in "test_ParseFixedPoint": check amount == 434238LL failed [-999999999999999999 != 434238]

This indicates to me that Core 0.12 would not accept the value you are trying to input either.

A negative amount being returned would indeed trigger an "Invalid amount" RPC error in sendmany().

I would say Core is doing the right thing with this change. By rejecting excessive precision values with an error rather than truncating or rounding them, they are avoiding the risk of invisibly making an irresponsible decision which would result in loss of information (since the input data cannot be represented precisely). This ensures that a user is informed and can take the appropriate decision.

 

[Steve Sokolowski]

Thanks for looking into this. You replied before I even came back to post more information, which shows how dedicated you are to your work.

While not your fault, this is a poor decision by the Core developers and is yet another example (along with Segregated Witness and replace-by-fee) of bad software engineering. I completely agree with you that, if this were being implemented for the first time, it absolutely makes sense to require that the decimals be rounded. But this has been out there for seven years, and there is no legitimate reason that the Core developers should be breaking compatibility like this, as the benefits of this change are far outweighed by all the people whose software has broken because of it.

I thought that one of the basic principles of development is don't make changes to production code unless they are absolutely necessary! There is no change that is too small to cause some major bug. We have a huge problem on our hands as this change is rebased into other coins, as we can't just mindlessly tinker around with payout code that we purposely haven't modified in two years.

Thanks for your help. At least we know for now that we can avoid this problem by not upgrading.