Implement fee inclusive transactions

[cliik]

This feature consists of three commits:

1. Extend API to allow "fee inclusive" transaction amounts
2. Expose this option in the CLI send command via new flag: --amount_includes_fee
3. Calculate & spend max wallet balance, if 'max' keyword is used as send amount

I've added tests for all three commits as well as functionally tested with real coins in a real wallet.

@yeastplume @phyro as written today, I think this incurs a breaking API change. I tried to come up with an approach that would be backwards compatible, but I kept having to resort to hacks and abuse of other args to make it happen... I decided it would be better to implement this logic "properly" for now, and see how bad the pushback is for making a breaking change to the API 😬. Please let me know your thoughts.

Resolves #601.

[cliik]

Sorry for the delay. This feature ended up being a lot more work than I expected! 😅

[yeastplume]

Once again, thanks for putting all of this together, and I can see all of the effort put into updating the unit and integration tests (which is where most of the work is). This all looks solidly put together and a very good example of the kind of PR I love to see as a first contribution to the project.

My only real comment is do we really need to break the API? If the amount_includes_fee field were an Option, it could be left out of all existing requests without breaking anything. That's generally been the approach when adding new fields.

[cliik]

My only real comment is do we really need to break the API? If the amount_includes_fee field were an Option, it could be left out of all existing requests without breaking anything. That's generally been the approach when adding new fields.

You are absolutely right. I think I had my wires crossed with another project, when I thought optional args weren't allowed in the API. I've revised it to use optional args now, so it should no longer be a breaking API change 🎉. Thanks for catching that!

[yeastplume]

All looks great now, thanks again for this!

[marekyggdrasil]

Hi, I'm wondering if this is part of any release? My 5.0.1 complains

Wallet command failed: Invalid Arguments: Could not parse amount as a number with optional decimal point. e=Amount string was invalid

after

grin-wallet send -d <address> max

[phyro]

@marekyggdrasil I don't think there's been a release done after this was merged. Most of the work on grin-wallet was work specific for grin-gui. My guess is that there are still some things to test on the GUI end which is perhaps why there was no release yet.