PHP 7.2 - json_encode() breaks transfer submission of certain amounts

[burnside]

I was having fits trying to get a certain transfer to go today. Tracking it down it turns out that an amount submitted for transfer as 0.06784267 was getting sent to the wallet daemon as 67842669999.99999.

Here's a log snippit:

Blockchain submitTransaction: address: xyz, amount: 0.06784267 ... {"jsonrpc":"2.0","method":"transfer","params":{"destinations":[{"amount":67842669999.99999,"address":"xyz"}],"mixin":6,"get_tx_key":true,"payment_id":"","account_index":0,"subaddr_indices":"","priority":1,"do_not_relay":false},"id":4}

Digging into it I discovered that PHP seems to like to take floats and do crazy things with them inside of json_encode(), so I figured somewhere along the line a conversion was happening and it was coming out a float before going into json_encode(). The culprit turned out to be walletRPC.php's _transform() method. Changing it from this:

public function _transform($amount = 0) { return $amount * 1000000000000; }

to this:

public function _transform($amount = 0) { return intval($amount * 1000000000000); }

managed to get me a transfer amount of 67842669999 ... still not right. So I ended up with this:

public function _transform($amount = 0) { return intval(bcmul($amount, 1000000000000)); }

and now everything works fine.

{"jsonrpc":"2.0","method":"transfer","params":{"destinations":[{"amount":67842670000,"address":"xyz"}],"mixin":6,"get_tx_key":true,"payment_id":"","account_index":0,"subaddr_indices":"","priority":1,"do_not_relay":false},"id":4}

Hope that helps someone!

[serhack]

Thank you, I'll commit this as soon as possible. However I'll try to investigate more about this.

[burnside]

You bet.

I suspect the results will be different in different PHP versions and possibly 32/64 bit differences

I was hopeful there may be a better way than adding a dependency on bcmath. (Not an issue for me, I'm already using it all over but maybe an issue for others.)

My next step would have been to try working with it as a string. I know json_encode won't convert strings. I tried the bcmath first because I wasn't sure if the monero rpc would take the amount as a string or not.

Cheers

[recanman]

I will test soon but I am considering bumping minimum version to PHP 7.4.

[BrianHenryIE]

Just bump it to 8! If anyone wants to continue working with 7.x they can use Rector.

[recanman]

I was planning on maintaining a legacy 7.x version, and a newer PHP 8 version. I'd rather keep 7.x support.