Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

streamlined the use of include_hex #12541

Closed
wants to merge 2 commits into from
Closed

Conversation

denis2342
Copy link

ScriptPubKeyToUniv is using include_hex now instead of 'true'
the printout of scriptsig is now also in a include_hex condition

ScriptPubKeyToUniv is using include_hex now
the printout of scriptsig is now also in a include_hex condition
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test could be added showing that decoderawtransaction response doesn't contains the hex fields.

Requires release note.

@@ -174,7 +174,8 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
in.pushKV("vout", (int64_t)txin.prevout.n);
UniValue o(UniValue::VOBJ);
o.pushKV("asm", ScriptToAsmStr(txin.scriptSig, true));
o.pushKV("hex", HexStr(txin.scriptSig.begin(), txin.scriptSig.end()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing keys can break existing software. Not sure if this needs deprecation a flag.

{
txnouttype type;
std::vector<CTxDestination> addresses;
int nRequired;

out.pushKV("asm", ScriptToAsmStr(scriptPubKey));
if (fIncludeHex)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, since you are updating style here, add { }?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in the second commit

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing here.

@promag
Copy link
Member

promag commented Feb 26, 2018

Here

TxToUniv(CTransaction(std::move(mtx)), uint256(), result, false);

include_hex is set to false.

@denis2342 denis2342 closed this Feb 27, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants