Details

Reviewers

jasonbcox
deadalnix
Fabien

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING67b8e462d2d4: Merge #10999: Fix amounts formatting in `decoderawtransaction`
rABC67b8e462d2d4: Merge #10999: Fix amounts formatting in `decoderawtransaction`

Summary

ce07638 doc: Add comment to use ValueFromAmount/AmountFromValue for JSON, not utilmoneystr (Wladimir J. van der Laan)
ec05c50 rpc: Use ValueFromAmount instead of FormatMoney in TxToUniv (Wladimir J. van der Laan)
46347ad rpc: Move ValueFromAmount to core_write (Wladimir J. van der Laan)
dac3782 doc: Correct AmountFromValue/ValueFromAmount names (Wladimir J. van der Laan)

Pull request description:

With this, the amounts returned in `decoderawtransaction` will be padded to 8 digits like anywhere else in the API.

This is accomplished by using `ValueFromAmount` in `TxToUniv`, instead of `FormatMoney` which it currently (mistakingly) uses. The `FormatMoney` function is only for debugging/logging use!

To avoid dependency issues, `ValueFromAmount` is moved to `core_write.cpp`, where it also fits better. I don't move `AmountFromValue` to `core_read.cpp` at the same time, as this would have more impact due to the RPCError dependency there.

(n.b.: large number of changed files is solely due to the util_tests JSONs needing update)

Tree-SHA512: 10fc2d27d33a77dbcb57aa7eccd4f53110c05d38eb7df6d40f10f14c08fad4274472e93af75aa59fe68ad0720fdf0930f0108124abef518e0dd162b3d2b2b292

Backport of Core PR10999
https://github.com/bitcoin/bitcoin/pull/10999

Test Plan

make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

nakihito created this revision.Jun 14 2019, 22:05

Owners added a reviewer: Restricted Owners Package.Jun 14 2019, 22:05

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 14 2019, 22:05

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6358: Diff 9462.Jun 14 2019, 22:16

jasonbcox requested changes to this revision.Jun 14 2019, 23:58

jasonbcox added inline comments.

src/core_io.h
8 ↗	(On Diff #9462)	Needs to be a bracketed include.
src/core_write.cpp
7 ↗	(On Diff #9462)	Why add this here?

This revision now requires changes to proceed.Jun 14 2019, 23:58

Removed extra include in core_write.cpp and bracketed include in core_io.h.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6361: Diff 9469.Jun 15 2019, 00:31

developer notes are missing.

src/core_io.h
8 ↗	(On Diff #9469)	This is not neede if you take the amount by ref (which is a signature change from the original, BTW).

This revision now requires changes to proceed.Jun 15 2019, 00:39

In D3344#78993, @deadalnix wrote:

developer notes are missing.

Looks like developer notes were added in this patch: https://reviews.bitcoinabc.org/D2941

Removed #include <amount.h> from core_io.h and changed ValueFromAmount() to pass amount by value rather than pass by reference.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6363: Diff 9471.Jun 15 2019, 01:21

deadalnix accepted this revision.Jun 15 2019, 21:07

Fabien accepted this revision.Jun 17 2019, 07:17

jasonbcox requested changes to this revision.Jun 17 2019, 17:32

jasonbcox added inline comments.

src/core_io.h
8 ↗	(On Diff #9469)	Correct, this isn't needed if passed by ref. But the original PR does pass by reference: https://github.com/bitcoin/bitcoin/pull/10999/files#diff-b624a5b31a81f5bbdcb9f452719067e8R30
11 ↗	(On Diff #9471)	This is correct.
30 ↗	(On Diff #9471)	Keep the original pass by reference.
src/core_write.cpp
20 ↗	(On Diff #9471)	ditto

This revision now requires changes to proceed.Jun 17 2019, 17:32

Changed amount to pass by reference again.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B6382: Diff 9499.Jun 17 2019, 18:52

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

jasonbcox accepted this revision.Jun 18 2019, 18:13

This revision is now accepted and ready to land.Jun 18 2019, 18:13

Closed by commit rABC67b8e462d2d4: Merge #10999: Fix amounts formatting in `decoderawtransaction` (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by nakihito). · Explain WhyJun 19 2019, 19:23

This revision was automatically updated to reflect the committed changes.

Nico Guiton <nico@bitframe.org> added a commit: rABC67b8e462d2d4: Merge #10999: Fix amounts formatting in `decoderawtransaction`.

Fabien mentioned this in D3419: Fix struct vs class mismatch for Amount.Jun 26 2019, 07:32

Fabien mentioned this in rABCe80c6110fd73: Fix struct vs class mismatch for Amount.Jun 26 2019, 16:08

Nico Guiton <nico@bitframe.org> added a commit: rSTAGING67b8e462d2d4: Merge #10999: Fix amounts formatting in `decoderawtransaction`.Sep 19 2019, 00:24

Fabien mentioned this in rSTAGINGe80c6110fd73: Fix struct vs class mismatch for Amount.Sep 19 2019, 00:35

Merge #10999: Fix amounts formatting in `decoderawtransaction`
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 9540

doc/release-notes.md

src/core_io.h

src/core_write.cpp

src/rpc/misc.cpp

src/rpc/net.cpp

src/rpc/server.h

src/rpc/server.cpp

src/test/rpc_tests.cpp

src/utilmoneystr.h

test/util/data/tt-delin1-out.json

test/util/data/tt-delout1-out.json

test/util/data/tt-locktime317000-out.json

test/util/data/txcreate1.json

test/util/data/txcreate2.json

test/util/data/txcreatedata1.json

test/util/data/txcreatedata2.json

test/util/data/txcreatedata_seq0.json

test/util/data/txcreatedata_seq1.json

test/util/data/txcreatemultisig1.json

test/util/data/txcreatemultisig2.json

test/util/data/txcreateoutpubkey1.json

test/util/data/txcreatescript1.json

test/util/data/txcreatescript2.json

test/util/data/txcreatesignv1.json

Merge #10999: Fix amounts formatting in `decoderawtransaction`ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 9540

doc/release-notes.md

src/core_io.h

src/core_write.cpp

src/rpc/misc.cpp

src/rpc/net.cpp

src/rpc/server.h

src/rpc/server.cpp

src/test/rpc_tests.cpp

src/utilmoneystr.h

test/util/data/tt-delin1-out.json

test/util/data/tt-delout1-out.json

test/util/data/tt-locktime317000-out.json

test/util/data/txcreate1.json

test/util/data/txcreate2.json

test/util/data/txcreatedata1.json

test/util/data/txcreatedata2.json

test/util/data/txcreatedata_seq0.json

test/util/data/txcreatedata_seq1.json

test/util/data/txcreatemultisig1.json

test/util/data/txcreatemultisig2.json

test/util/data/txcreateoutpubkey1.json

test/util/data/txcreatescript1.json

test/util/data/txcreatescript2.json

test/util/data/txcreatesignv1.json

Merge #10999: Fix amounts formatting in `decoderawtransaction`
ClosedPublic
Actions

Revision Contents
Changeset List