[RPC] Only return hex field once in getrawtransaction #11027

achow101 · 2017-08-10T23:08:49Z

The hex is already returned in TxToUniv(), no need to give it out a second time in getrawtransaction itself.

The hex is already returned in TxToUniv, no need to give it out a second independent time in getrawtransaction itself.

sdaftuar · 2017-08-11T12:21:45Z

ACK

maflcko · 2017-08-11T13:21:16Z

utACK e029c6e, as this is a bug fix. Though, could this potentially break client when -rpcserialversion=0? I am thinking about how different json implementations handle duplicate keys in a dict, where each key points to a different value.

achow101 · 2017-08-11T18:54:51Z

@MarcoFalke Hmm. That might be a problem since RPCSerializationFlags are not passed to EncodeHexTx in TxToUniv.

sipa · 2017-08-11T19:00:29Z

That might be a problem since RPCSerializationFlags are not passed to EncodeHexTx in TxToUniv.

That seems to be a bug on its own.

achow101 · 2017-08-11T19:05:17Z

Another thing is that not everything that uses TxToUniv needs to have hex output, e.g. decoderawtransaction

achow101 · 2017-08-11T19:22:23Z

I added a commit which passes the RPCSerializationFlags to TxToUniv and a boolean for whether to give hex output or not.

sipa

utACK

sipa · 2017-08-11T20:01:12Z

src/core_io.h

Marking a by-value parameter as const is redundant. Also, style nit on serializeFlags.

Fixed. It was copy and pasted from two lines above in EncodeHexTx :)

maflcko · 2017-08-12T00:18:28Z

That seems to be a bug on its own.

I don't think so?

According to the doc, -rpcserialversion sets the serialization of raw transaction or block hex returned in non-verbose mode.

promag

utACK 9381d86.

There are only 2 more calls to TxToUniv so I wonder if it pays to have default arguments.

promag · 2017-08-11T23:59:59Z

src/core_io.h

I think the order should be inverted. There is no need to pass serialize_flags if one wants to include_hex=false.

~~Don't care about the order, though I'd prefer not using default arguments. Easy enough to just add these on all call sites, there aren't many.~~

Edit: never mind, I think this is a good use of default arguments, just swap them as @promag says.

achow101 · 2017-08-12T00:28:13Z

@MarcoFalke I interpret that as talking about when verbose mode means that you wouldn't get the hex back, e.g. in getblock. If we are returning hex, then I think we should be consistent and all hex should follow whatever the RPCSerializationFlags are.

gmaxwell

ACK. This needs a 0.15 tag: it's a 0.15 regression and it results in invalid JSON.

jnewbery · 2017-08-17T17:19:17Z

Strictly speaking, I don't think this is invalid JSON, although client behaviour is undefined if object names are not unique (https://tools.ietf.org/html/rfc7159#section-4). But yes, this is a regression in 0.15 and should be treated as a bug fix.

Perhaps we should add JSON object checking to rpcserver.cpp (in JSONRPCExecOne()) and/or authproxy.py (in __call__()) to verify the validity of our returned JSON.

jnewbery · 2017-08-17T17:33:33Z

Concept ACK

Another thing is that not everything that uses TxToUniv needs to have hex output, e.g. decoderawtransaction
I added a commit which passes ... a boolean for whether to give hex output or not.

I don't think you should complicate the interface to TxToUniv to specify which fields to return. Either filter them out at the caller, or just return the hex in decoderawtransaction. Is there any harm in returning the hex in that RPC?

achow101 · 2017-08-17T17:52:59Z

Swapped the arguments.

Either filter them out at the caller

I looked at that but I couldn't find anything in Univalue for removing things from it.

or just return the hex in decoderawtransaction. Is there any harm in returning the hex in that RPC?

Since you are already providing it the hex, it's not useful to be giving the hex back out to the user.

jtimon · 2017-08-17T20:41:36Z

utACK e029c6e

There are only 2 more calls to TxToUniv so I wonder if it pays to have default arguments.

+1

promag · 2017-08-17T22:54:51Z

Wild option 🙄:

void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry);
void TxToUnivWithHex(const CTransaction& tx, const uint256& hashBlock, int serialize_flags, UniValue& entry);

laanwj · 2017-08-21T06:41:40Z

Since you are already providing it the hex, it's not useful to be giving the hex back out to the user.

I was also really surprised to see the hex being echoed back in decoderawtransaction output. That's just a waste of bandwidth especially for large transactions.

Either filter them out at the caller

No, don't do this. univalue objects aren't meant to be written to/processed after initial generation. The library is not optimized for that. If you need any kind of editing use an intermedaite data structure (which makes no sense from 'XXXToJSON` functions). Specifying which fields to include looks like exactly the right way here.

6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262

The hex is already returned in TxToUniv, no need to give it out a second independent time in getrawtransaction itself. Github-Pull: #11027 Rebased-From: e029c6e Tree-SHA512: 4f7892431ddb9b59bcc59756890b97a20d046d1d4898f8a80c564223e1cde1c922da403cea4b8ae79d70741d1a9ff337f077043e5bb538cdc4d34fbe09301240

Github-Pull: #11027 Rebased-From: 6bbdafc Tree-SHA512: 8ed049a0945c4f56c518aef6d60efebda5cde09ef63d8e472b363821170d71fb4d16414b360824d7537fc7c7b5e5277bbbd092a20899446e97c188ab8df58c1b

Summary: 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262 Backport of Core PR 11027 bitcoin/bitcoin#11027 Completes T564 Test Plan: ninja check test_runner.py Run `bitcoin-cli <txid> 1` and ensure that there is a single top-level `"hex"` field in the returned JSON. Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2709

Summary: 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262 Backport of Core PR 11027 bitcoin/bitcoin#11027 Completes T564 Test Plan: ninja check test_runner.py Run `bitcoin-cli <txid> 1` and ensure that there is a single top-level `"hex"` field in the returned JSON. Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2709

@luke-jr

* test: Replace remaining sprintf with snprintf Summary: Use of `sprintf` is seen as a red flag as many of its uses are insecure. OpenBSD warns about it while compiling, and some modern platforms, e.g. [cloudlibc from cloudabi](https://github.com/NuxiNL/cloudlibc) don't even provide it anymore. Backport of core PR9867 Test Plan: make check Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, deadalnix, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2697 * Merge #10783: [RPC] Various rpc argument fixes Summary: 4dc1915 check for null values in rpc args and handle appropriately (Gregory Sanders) 999ef20 importmulti options are optional (Gregory Sanders) a70d025 fixup some rpc param counting for rpc help (Gregory Sanders) Pull request description: Audited where named args will fail to use correct default values or may fail when additional optional arguments are added. Previously for these parameters, it was fine to omit them as positional arguments, but it would trigger UniValue runtime errors to set them to null, or to omit them while passing named parameters with greater positions (which would internally set earlier missing arguments to null). Now null values are treated the same as missing values so these errors do not occur. Included a few other small fixes while working on it. I didn't bother fixing account-based rpc calls. Tree-SHA512: 8baf781a35bd48de7878d4726850a580dab80323d3416c1c146b4fa9062f8a233c03f37e8ae3f3159e9d04a8f39c326627ca64c14e1cb7ce72538f934ab2ae1e Backport of Core PR 10783 https://github.com/bitcoin/bitcoin/pull/10783/files Completes T552 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2688 * Merge #10775: nCheckDepth chain height fix Summary: d9d1bd3 nCheckDepth chain height fix (romanornr) Pull request description: ```` if (nCheckDepth <= 0) nCheckDepth = 1000000000; // suffices until the year 19000 if (nCheckDepth > chainActive.Height()) nCheckDepth = chainActive.Height(); ```` These lines confuse me. Correct me if I am wrong, but we can't check any more blocks than we have right? If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement. ```` if (nCheckDepth > chainActive.Height()) nCheckDepth = chainActive.Height(); ```` when using ````--checkblocks=Z```` When Z is ````0```` or any other negative number, it will check all blocks. I think it should be changed to this maybe. ```` if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height()) nCheckDepth = chainActive.Height(); ```` Which gets rid of that huge number which is confusing for any other altcoins that have a different block time. Tree-SHA512: 8ee0ae5f33b399fa74dc16926709694ccfe1fc8a043cba2f5d00884220ac1b9b13f2df4588041f4133be634e5c7b14f4eebe24294028dafe91581a97dbe627f3 Backport of Core PR 10775 https://github.com/bitcoin/bitcoin/pull/10775/files Test Plan: ninja check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2691 * Merge #9622: [rpc] listsinceblock should include lost transactions when parameter is a reorg'd block Summary: 876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm) f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm) Pull request description: The following scenario will not notify the caller of the fact `tx0` has been dropped: 1. User 1 receives BTC in tx0 from utxo1 in block aa1. 2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1 3. User 1 sees 2 confirmations at block aa3. 4. Reorg into bb chain. 5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated. See `listsinceblock.py` commit for related test. The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe. Example output: ```Python { 'transactions': [], 'replaced': [ { 'walletconflicts': [], 'vout': 1, 'account': '', 'timereceived': 1485234857, 'time': 1485234857, 'amount': '1.00000000', 'bip125-replaceable': 'unknown', 'trusted': False, 'category': 'receive', 'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff', 'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ', 'label': '', 'confirmations': -7 } ], 'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715' } ``` I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong.. Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35 Backport of Core PR 9622 https://github.com/bitcoin/bitcoin/pull/9622/files Completes T551 Test Plan: make check test_runner.py wallet_listsinceblock Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2689 * Merge #11039: Avoid second mapWallet lookup Summary: 8f2f1e0 wallet: Avoid second mapWallet lookup (João Barbosa) Pull request description: All calls to `mapWallet.count()` have the intent to detect if a `txid` exists and most are followed by a second lookup to retrieve the `CWalletTx`. This PR replaces all `mapWallet.count()` calls with `mapWallet.find()` to avoid the second lookup. Tree-SHA512: 96b7de7f5520ebf789a1aec1949a4e9c74e13683869cee012f717e5be8e51097d068e2347a36e89097c9a89f1ed1a1529db71760dac9b572e36a3e9ac1155f29 Backport of Core PR 11039 https://github.com/bitcoin/bitcoin/pull/11039/files Completes T550 Depends on D2689 Test Plan: ninja check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2690 * Merge #11027: [RPC] Only return hex field once in getrawtransaction Summary: 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262 Backport of Core PR 11027 bitcoin/bitcoin#11027 Completes T564 Test Plan: ninja check test_runner.py Run `bitcoin-cli <txid> 1` and ensure that there is a single top-level `"hex"` field in the returned JSON. Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2709 * Merge #11565: Make listsinceblock refuse unknown block hash Summary: 659b206 Make listsinceblock refuse unknown block hash (Russell Yanofsky) Pull request description: Change suggested by @theuni who noticed listsinceblock would ignore invalid block hashes causing it to return a completely unfiltered list of transactions. Tree-SHA512: 3c8fb160265780d1334e856e853ab48e2e18372b8f1fc71ae480c3f45317048cc1fee0055d5c58031981a91b9c2bdbeb8e49a889d04ecba61729ce8109f2ce3f Backport of Core PR 11565 https://github.com/bitcoin/bitcoin/pull/11565/files Completes T563 Test Plan: ninja check wallet_listsinceblock.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2703 * Merge #11618: rpc: Lock cs_main in blockToJSON/blockheaderToJSON Summary: a9b6ba0b7 Add missing cs_main locks when calling blockToJSON/blockheaderToJSON (practicalswift) Pull request description: `blockToJSON(...)` and `blockheaderToJSON(...)` read the variable `chainActive` which requires holding the mutex `cs_main`. So does `GetDifficulty(...)`. Tree-SHA512: bfb94f5e3238accbf6a4daddde49d53f1891c38ae9b07e25b3098c485747159258f64bb66a50e147b32beac601de89d9d04ff717b6c4f1460d329c90a53d3333 Backport of Core PR 11618 https://github.com/bitcoin/bitcoin/pull/11618/files Test Plan: make check test_runner.py bitcoin-cli getblock 000000000933ea01ad0ee984209779baaec3ced90fa3f408719526f8d77f4943 Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2684 * Remove redundant locks Summary: * SetAddressBook(...) is locking cs_wallet internally * DelAddressBook(...) is locking cs_wallet internally Backport of PR11733 bitcoin/bitcoin#11733 Test Plan: make check test_runner.py Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2700 * Merge #12327: [gui] Defer coin control instancing Summary: 6558f8acc [gui] Defer coin control instancing (João Barbosa) Pull request description: Defer the GUI coin control instancing so that argument processing is taken into account for the default coin control values. Fixes #12312 Tree-SHA512: ecda28b94f4709319e9484b01afe763c7c3569097d2afb89db79da8a195c46d20ea77166df7edce0c8ab77627b295def01c072148714503436d27675d5e75d99 Backport of Core PR 12327 https://github.com/bitcoin/bitcoin/pull/12327/files Completes T556 Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2693 * [LINTER] Improve check-doc regex Summary: Improve the regex: - Use Perl type regex in grep to remove duplication. Previously there was 2 alsmost identical regex, one posix compliant for egrep and one Perl compliant for python re. - Exclude the test folder from the source path rather than subtracting its matching output This improvements make the `-h` now detectable; it is added to the undocumented exceptions. Inspired from core PR12820. Note that GNU `grep` is used instead of `git grep` to allow for multiline regex (required due to our code format). Test Plan: Should return no error: arc lint ./test/lint/check-doc.py There should be 1 more used argument than before this patch (`-h`), otherwise the output should be identical. Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2694 * [LINTER] Fix check doc incompatibility with BSD grep Summary: BSD grep on OSX and FreeBSD does not provide the same options as GNU grep which prevent using them for multiline search. This diff uses only python regex to avoid this incompatibility. As a bonus, it runs about 3x faster. Depends on D2694 Test Plan: Should return no error: arc lint In `src/init.cpp` delete the line 1279: ``` logger.m_print_to_console = gArgs.GetBoolArg("-printtoconsole", false); ``` Then run `arc lint` and check the linter outputs an Unknown argument error: '-printtoconsole' is documented but not used' Restore the previously deleted line in `src/init.cpp` and now delete the lines 792 to 794: ``` strUsage += HelpMessageOpt( "-printtoconsole", _("Send trace/debug info to console instead of debug.log file")); ``` Then run `arc lint` and check the linter outputs an Undocumented argument error: '-printtoconsole' is undocumented' Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2695 * Backport RetFormat enum class Summary: Partial backport of Core PR 10742 (RetFormat only) https://github.com/bitcoin/bitcoin/pull/10742/files Progess towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2712 * Backport BlockSource enum class Summary: Partial backport of Core PR 10742 (BlockSource only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2713 * Backport HelpMessageMode enum class Summary: Partial backport of Core PR 10742 (HelpMessageMode only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2714 * Merge #8665: Assert all the things! Summary: 4d51e9b Assert ConnectBlock block and pIndex are the same block (NicolasDorier) 972714c pow: GetNextWorkRequired never called with NULL pindexLast (Daniel Cousens) cc44c8f ContextualCheckBlockHeader should never have pindexPrev to NULL (NicolasDorier) Tree-SHA512: 7cc568bf9417267c335f21ec3d1505b26e56e5b3d5f4d3dbb555279489800aaa65a3bcd7bc376e274dd102912aec16ddbb18de2e2060b2667b41eb979cd9321e Backport of Core PR 8665 https://github.com/bitcoin/bitcoin/pull/8665/files Test Plan: make check test_runner.py Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2722 * Merge #11028: Avoid masking of difficulty adjustment errors by checkpoints Summary: 85c82b5 Avoid masking of difficulty adjustment errors by checkpoints (Pieter Wuille) Pull request description: Currently difficulty adjustment violations are not reported for chains that branch off before the last checkpoint. Change this by moving the checkpoint check after the difficulty check. Tree-SHA512: 33666f2c3459151b28c42041a463779e6df18f61d3dd5b1879a0af4e5b199ef74d1e33e06af68bebfdfb211569ad5fb56556bfebe9d63b5688d910ea211b839a Backport of Core PR 11028 https://github.com/bitcoin/bitcoin/pull/11028/files Depends on D2722 Test Plan: ``` make check test_runner.py ``` Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2723 * Avoid slow transaction search with txindex enabled Summary: Backport of PR11529: bitcoin/bitcoin#11529 Completes T565 Test Plan: make check test_runner.py Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2710 * [secp256k1] fix java secp256k1 test Summary: fix java secp256k1 test Test Plan: to run the java secp256k1 tests (including this one): ``` $ cd src/secp256k1 $ ./autogen.sh $ ./configure --enable-jni --enable-experimental --enable-module-ecdh $ make check-java ``` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox Subscribers: jasonbcox, teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2685 * Backport FlushStateMode enum class Summary: Partial backport of Core PR 10742 (FlushStateMode only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2716 * Allow for running secp256k1 java build/tests out of tree Summary: As per title Test Plan: mkdir -p src/secp256k1/build cd src/secp256k1 mkdir -p src/java/guava wget https://search.maven.org/remotecontent?filepath=com/google/guava/guava/18.0/guava-18.0.jar -O src/java/guava/guava-18.0.jar ./autogen.sh cd build ../configure --enable-jni --enable-experimental --enable-module-ecdh make check-java (`make check-java` may pass or fail depending of D2685) Use `make clean` to remove the .class files generated by `javac`. Reviewers: #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2717 * Backport VerifyResult enum class Summary: Partial backport of Core PR 10742 (VerifyResult only) https://github.com/bitcoin/bitcoin/pull/10742/files Progress towards T549 Test Plan: ninja check Reviewers: deadalnix, Fabien, #bitcoin_abc Reviewed By: Fabien, #bitcoin_abc Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2718 * Remove unused seeder/compat.h Summary: This file is no longer used in the seeder since it is included in the main tree (src/compat.h is used). Test Plan: ./autogen.sh mkdir build && cd build ../configure make ./src/bitcoin-seeder Ensure the software runs as expected cd .. mkdir buildcmake && cd buildcmake cmake -GNinja .. ninja ./src/bitcoin-seeder Ensure the software runs as expected Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2728 * [secp256k1] remove guava dep Summary: remove overkill depencency to guava Test Plan: execute previous tests successfully the JNI part can be built and tested by simply doing ``` $ ./autogen.sh $ ./configure --enable-jni --enable-experimental --enable-module-ecdh $ make check-java ``` Once https://reviews.bitcoinabc.org/D2686 is passed we can do this automatically Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien Subscribers: Fabien, teamcity, schancel Differential Revision: https://reviews.bitcoinabc.org/D2681 * [secp256k1] remove unused byte array Summary: remove unused bytearray in verify Test Plan: re run tests Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: schancel Differential Revision: https://reviews.bitcoinabc.org/D2734 * Fixup merge issue * Remove seeder.h from make

…ction 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262

…nsaction (#3298) * Merge bitcoin#11027: [RPC] Only return hex field once in getrawtransaction 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262 * Remove backported traces of RPCSerializationFlags We don't have this in Dash Co-authored-by: Wladimir J. van der Laan <[email protected]>

…nsaction (dashpay#3298) * Merge bitcoin#11027: [RPC] Only return hex field once in getrawtransaction 6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow) e029c6e Only return hex field once in getrawtransaction (Andrew Chow) Pull request description: The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself. Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262 * Remove backported traces of RPCSerializationFlags We don't have this in Dash Co-authored-by: Wladimir J. van der Laan <[email protected]>

Only return hex field once in getrawtransaction

e029c6e

The hex is already returned in TxToUniv, no need to give it out a second independent time in getrawtransaction itself.

fanquake added the RPC/REST/ZMQ label Aug 10, 2017

sipa reviewed Aug 11, 2017

View reviewed changes

achow101 force-pushed the fix-getrawtx branch from eb1dcaa to 9381d86 Compare August 11, 2017 20:11

promag reviewed Aug 12, 2017

View reviewed changes

gmaxwell approved these changes Aug 14, 2017

View reviewed changes

sipa added the Bug label Aug 14, 2017

sipa added this to the 0.15.0 milestone Aug 14, 2017

maflcko added the Needs backport label Aug 16, 2017

Pass serialization flags and whether to include hex to TxToUniv

6bbdafc

achow101 force-pushed the fix-getrawtx branch from 9381d86 to 6bbdafc Compare August 17, 2017 17:52

laanwj merged commit 6bbdafc into bitcoin:master Aug 21, 2017

laanwj removed the Needs backport label Aug 21, 2017

achow101 deleted the fix-getrawtx branch August 29, 2017 15:27

codablock mentioned this pull request Jan 21, 2020

Backport bitcoin#11027: [RPC] Only return hex field once in getrawtransaction dashpay/dash#3298

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

[RPC] Only return hex field once in getrawtransaction #11027

[RPC] Only return hex field once in getrawtransaction #11027

Uh oh!

Conversation

achow101 commented Aug 10, 2017

Uh oh!

sdaftuar commented Aug 11, 2017

Uh oh!

maflcko commented Aug 11, 2017

Uh oh!

achow101 commented Aug 11, 2017

Uh oh!

sipa commented Aug 11, 2017

Uh oh!

achow101 commented Aug 11, 2017

Uh oh!

achow101 commented Aug 11, 2017

Uh oh!

sipa left a comment

Choose a reason for hiding this comment

Uh oh!

sipa Aug 11, 2017

Choose a reason for hiding this comment

Uh oh!

achow101 Aug 11, 2017

Choose a reason for hiding this comment

Uh oh!

maflcko commented Aug 12, 2017

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag Aug 11, 2017

Choose a reason for hiding this comment

Uh oh!

laanwj Aug 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achow101 commented Aug 12, 2017

Uh oh!

gmaxwell left a comment

Choose a reason for hiding this comment

Uh oh!

jnewbery commented Aug 17, 2017

Uh oh!

jnewbery commented Aug 17, 2017

Uh oh!

achow101 commented Aug 17, 2017

Uh oh!

jtimon commented Aug 17, 2017

Uh oh!

promag commented Aug 17, 2017

Uh oh!

laanwj commented Aug 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

laanwj Aug 17, 2017 •

edited

Loading

laanwj commented Aug 21, 2017 •

edited

Loading