update bitcoin-tx to output witness data #3
Conversation
sdaftuar
left a comment
sdaftuar
left a comment
There was a problem hiding this comment.
utACK apart from the one style nit -- I agree with you that it seems like we should use this function in the RPC call as well. Maybe when you open a PR upstream you should ask if it'd be better to add an additional commit that does the deduplication you mentioned? (I haven't checked to see if there would be any changes to the output by doing so, but I can't think of why it would be a problem to de-duplicate the code somehow.)
src/core_write.cpp
Outdated
There was a problem hiding this comment.
Fixed (and changed push_back(Pair()) to pushKV to match other fields).
All test_bitcoin tests pass.
|
I've addressed @sdaftuar's code nit and also added the witness hash to the Suhas, @TheBlueMatt , do you have any further comments before I open a PR against https://github.com/bitcoin/bitcoin/ ? I'm planning to open this PR and also #4 to deduplicate the |
|
utACK. |
jnewbery
force-pushed
the
bitcoin-tx-witness
branch
from
8cea1b8 to
76992bf
Compare
|
Thanks. All squashed. I'll open a PR to bitcoin/bitcoin. |
|
I've opened a PR to bitcoin/bitcoin. Closing this PR. |
16a1f7f Merge #3: Pull upstream 3f03bfd Merge pull request #27 from laanwj/2016_09_const_refs 5668ca3 Return const references from getKeys, getValues, get_str cedda14 Merge pull request #28 from MarcoFalke/patch-1 9f0b997 [travis] Work around osx libtool issue git-subtree-dir: src/univalue git-subtree-split: 16a1f7f6e9ed932bec7c7855003affea1e165fb5
…ter-return checking 8d22ab0 ci: Enable address sanitizer (ASan) stack-use-after-return checking (practicalswift) Pull request description: Enable address sanitizer (ASan) stack-use-after-return checking (`detect_stack_use_after_return=1`). Example: ``` #include <iostream> #include <string> const std::string& get_string(int i) { return std::to_string(i); } int main() { std::cout << get_string(41) << "\n"; } ``` Without address sanitizer (ASan) stack-use-after-return checking: ``` $ ./stack-use-after-return $ ``` With address sanitizer (ASan) stack-use-after-return checking: ``` $ ASAN_OPTIONS="detect_stack_use_after_return=1" ./stack-use-after-return ================================================================= ==10400==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f7fa0400030 at pc 0x00000049d2cc bp 0x7ffcbd617070 sp 0x7ffcbd616820 READ of size 2 at 0x7f7abbecd030 thread T0 #0 0x439781 in fwrite #1 0x7f7ac0504cb3 in std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x113cb3) #2 0x4f9b5f in main stack-use-after-return.cpp:9:15 #3 0x7f7abf440b96 in __libc_start_main #4 0x41bbc9 in _start … $ ``` Top commit has no ACKs. Tree-SHA512: 6557a9ff184023380fd9aa433cdf413e01a928ea99dbc59ec138e5d69cb9e13592e8bb5951612f231ff17a37a895bec5c0940c8db5f328a5c840a5771bdeeba5
3 participants
In bitcoin@7c4bf77 , jl2012 updated getrawtransaction() to return the witness data for transaction inputs spending P2WPK and P2WSH.
bitcoin-tx doesn't call into this function and instead calls
TxToUniv()incore_write.cpp, so bitcoin-tx calls don't return witness data.I think the proper fix is to have
getrawtransaction()call intoTxToUniv()so we don't have duplicate code. For now, I'm just adding code toTxToUniv()to return witness data.