Add new "bitcoin-tx" command line utility #4332

jgarzik commented at 4:10 AM on June 12, 2014: contributor

This is a simple utility that provides command line manipulation of a hex-encoded TX. The utility takes a hex string on the command line as input, performs zero or more mutations, and outputs a hex string to standard output.

This utility is also an intentional exercise of the "bitcoin library" concept. It is designed to require minimal libraries, and works entirely without need for any RPC or P2P communication.

The command line help provides a list of the mutations available:

Usage:
  rawtx [options] <hex-tx> [commands]  Update hex-encoded bitcoin transaction
  rawtx [options] -create [commands]   Create hex-encoded bitcoin transaction

Options:
  -?                      This help message
  -create                 Create new, empty TX.
  -json                   Select JSON output
  -regtest                Enter regression test mode,instantly.
  -testnet                Use the test network

Commands:
  delin=N                Delete input N from TX
  delout=N               Delete output N from TX
  in=TXID:VOUT           Add input to TX
  locktime=N             Set TX lock time to N
  nversion=N             Set TX version to N
  outaddr=VALUE:ADDRESS  Add address-based output to TX
  outscript=VALUE:SCRIPT Add raw script output to TX
  sign=SIGHASH-FLAGS     Add zero or more signatures to transaction
      This command requires JSON registers:
      prevtxs=JSON object
      privatekeys=JSON object
      See signrawtransaction docs for format of sighash flags, JSON objects.

Register Commands:
  load.NAME=FILE         Load JSON file FILE into register NAME
  NAME=JSON-STRING       Set register NAME to given JSON-STRING

theuni commented at 5:25 AM on June 12, 2014: member

Since you pinged earlier, ACK on the buildsystem changes.

And noted as a good example of a lib-refactor working case.

jgarzik commented at 5:33 AM on June 12, 2014: contributor

Interesting that pulltester pukes on "replace_first", but builds for me. Will update, along with some more goodies.

Everything submitted builds and tests locally, of course.

theuni commented at 5:56 AM on June 12, 2014: member

@jgarzik I'd guess that pull-tester's version of boost doesn't rope in that header as freely. src/test/script_tests.cpp, uses replace_first as well, and includes:

#include <boost/algorithm/string/replace.hpp>

jgarzik commented at 3:26 PM on June 12, 2014: contributor

Added JSON output (a la decoderawtransaction). Notably, this uses a new "UniValue" (universal value) class to provide JSON containers and output, at a fraction of the cost of JSON-spirit.

sipa commented at 10:40 PM on June 12, 2014: member

Suggestion: separate commands from options:

./rawtx --flag1 --flag2 [transaction] command1=arg command2=arg

So -regtest, -testnet, -? remain flags, but the rest are operations to be performed in order on the transaction data.

jgarzik commented at 2:40 AM on June 13, 2014: contributor

Rebased. univalue JSON parser passes the "vjson" project's test suite. @sipa Seems reasonable.

jgarzik commented at 7:41 PM on June 13, 2014: contributor

Updated the command line UI per @sipa suggestion. That simplified the utility a bit. Updated OP to reflect new --help output and usage.

in src/rawtx.cpp:None in 6a8fa88c1f outdated

 163 | +}
 164 | +
 165 | +static void MutateTxVersion(CTransaction& tx, const string& cmdVal)
 166 | +{
 167 | +    int64_t newVersion = atoi64(cmdVal);
 168 | +    if (newVersion < 1 || newVersion > 2)

sipa commented at 8:49 PM on June 13, 2014:

There is no such thing as a transaction version 2, afaik. Also, use CTransaction::CURRENT_VERSION instead of a hardcoded maximum?

jgarzik commented at 8:51 PM on June 13, 2014:

The general idea was CURRENT_VERSION+1, which I probably should have made more explicit, with the thought of perhaps enabling some experimentation. Maybe that's a dumb idea, and it should never exceed CURRENT_VERSION [without an easy and obvious hack by a programmer].

sipa commented at 8:59 PM on June 13, 2014:

Transactions with nVersion > CURRENT_VERSION are non-standard anyway, so they wouldn't be relayed by even your own software.

in src/rawtx.cpp:None in 6a8fa88c1f outdated

 172 | +}
 173 | +
 174 | +static void MutateTxLocktime(CTransaction& tx, const string& cmdVal)
 175 | +{
 176 | +    int64_t newLocktime = atoi64(cmdVal);
 177 | +    if (newLocktime < 250000LL || newLocktime > 0xffffffffLL)

sipa commented at 8:51 PM on June 13, 2014:

What makes block 250000 special? Why is it hardcoded?

jgarzik commented at 8:53 PM on June 13, 2014:

Absolutely nothing. It's a sanity check that may want removing. The thought was to prevent a pointlessly low locktime, but whatever, not important and probably wrong.

in src/rawtx.cpp:None in 6a8fa88c1f outdated

 193 | +        throw runtime_error("invalid TX input txid");
 194 | +    uint256 txid(strTxid);
 195 | +
 196 | +    string strVout = strInput.substr(pos + 1, string::npos);
 197 | +    int vout = atoi(strVout);
 198 | +    if ((vout < 0) || (vout > 100000))

sipa commented at 8:53 PM on June 13, 2014:

Suggestion: use (MAX_BLOCK_SIZE / CTxOut()::GetSerializeSize(SER_NETWORK, PROTOCOL_VERSION)) as maximum instead of a hardcoded 100000?

jgarzik commented at 6:01 PM on June 14, 2014: contributor

Updated to improve guards on user inputs per @sipa, and to add transaction signing. Updated --help output in OP.

rawtx should be functionally equivalent to the RPC raw transaction API, when bitcoind is compiled with --disable-wallet.

There is some amount of almost-duplicate code, that can be harmonized once the RPC server is switched from JSON-spirit to UniValue.

jgarzik commented at 6:03 PM on June 14, 2014: contributor

PS. Considering renaming util to "bitcoin-tx" to match existing binary naming, and be a bit more friendly to the global /usr/bin namespace.

theuni commented at 11:13 PM on June 14, 2014: member

+1 on renaming

laanwj commented at 1:08 PM on June 15, 2014: member

As a tool this looks useful and can eventually take over the pure-utility subset of the RPC interface.

+1 on renaming. Unless we still intend to rename the executables to bitcoin-core-... or such, but that makes the names long and unwieldy and everyone is used to the current bitcoin-*.

BTW: I've cherry-picked the first commit (as 0cafb63), it seems unrelated to the rest of this pull.

jgarzik commented at 6:27 PM on June 15, 2014: contributor

Rebased. Renamed util to bitcoin-tx.

jgarzik renamed this:
~~Add new "rawtx" command line utility~~
Add new "bitcoin-tx" command line utility
on Jun 15, 2014

sipa commented at 10:19 AM on June 16, 2014: member

Do you plan to add support for signing as well (where you pass in the utxo's being spent, and the private keys)?

jgarzik commented at 12:17 PM on June 16, 2014: contributor

@sipa Yes, that is already supported. Added the "sign" command yesterday.

jgarzik commented at 10:19 PM on June 20, 2014: contributor

This should be ready for merging, unless we want to add "remove JSON-SPIRIT" as a blocker.

theuni commented at 2:47 AM on June 21, 2014: member

@jgarzik I saw that you requested i nitpick the univalue class. Will look over it on Monday if I don't get a chance over the weekend.

laanwj commented at 7:55 AM on June 21, 2014: member

As it is an independent library, I think it would serve modularity (and in any case reduce the number of files in the src root) to put the univalue implementation into a subdirectory of src. "remove JSON-SPIRIT" is not a blocker, I really prefer that in a separate pull request later on.

in src/bitcoin-tx.cpp:None in 480ec93fbb outdated

 109 | +        char buf[4096];
 110 | +        int bread = fread(buf, 1, sizeof(buf), f);
 111 | +        if (!bread)
 112 | +            break;
 113 | +
 114 | +        string tmpStr(buf, bread);

theuni commented at 7:24 PM on June 23, 2014:

valStr.insert() should save an allocation/copy?

in src/univalue.cpp:None in 480ec93fbb outdated

  28 | +    clear();
  29 | +    typ = (val ? VTRUE : VFALSE);
  30 | +    return true;
  31 | +}
  32 | +
  33 | +static bool validNumStr(const string& s)

theuni commented at 7:30 PM on June 23, 2014:

s.find_first_of()

theuni commented at 7:32 PM on June 23, 2014:

also, anon namespace would be nice.

in src/univalue.cpp:None in 480ec93fbb outdated

  60 | +    }
  61 | +
  62 | +    return true;
  63 | +}
  64 | +
  65 | +bool UniValue::setNumStr(string val_)

theuni commented at 7:33 PM on June 23, 2014:

const std::string& val_?

in src/univalue.cpp:None in 480ec93fbb outdated

  91 | +    oss << val;
  92 | +
  93 | +    return setNumStr(oss.str());
  94 | +}
  95 | +
  96 | +bool UniValue::setStr(string val_)

theuni commented at 7:35 PM on June 23, 2014:

const std::string& val_

in src/univalue.cpp:None in 480ec93fbb outdated

 113 | +    clear();
 114 | +    typ = VOBJ;
 115 | +    return true;
 116 | +}
 117 | +
 118 | +bool UniValue::push(UniValue& val)

theuni commented at 7:35 PM on June 23, 2014:

const UniValue&

in src/univalue.cpp:None in 480ec93fbb outdated

 122 | +
 123 | +    values.push_back(val);
 124 | +    return true;
 125 | +}
 126 | +
 127 | +bool UniValue::pushKV(string key, UniValue& val)

theuni commented at 7:38 PM on June 23, 2014:

const std::string& key and const val. I won't spam you with any more of these.

in src/core_read.cpp:None in 480ec93fbb outdated

  17 | +{
  18 | +    CScript result;
  19 | +
  20 | +    static map<string, opcodetype> mapOpNames;
  21 | +
  22 | +    if (mapOpNames.size() == 0)

theuni commented at 7:51 PM on June 23, 2014:

.empty()

in src/core_read.cpp:None in 480ec93fbb outdated

  19 | +
  20 | +    static map<string, opcodetype> mapOpNames;
  21 | +
  22 | +    if (mapOpNames.size() == 0)
  23 | +    {
  24 | +        for (int op = 0; op <= OP_NOP10; op++)

theuni commented at 7:52 PM on June 23, 2014:

opcodetype for clarity?

in src/core_read.cpp:None in 480ec93fbb outdated

  52 | +        {
  53 | +            // Number
  54 | +            int64_t n = atoi64(w);
  55 | +            result << n;
  56 | +        }
  57 | +        else if (starts_with(w, "0x") && IsHex(string(w.begin()+2, w.end())))

theuni commented at 8:02 PM on June 23, 2014:

... && w.size() >= 2 && IsHex ... sanity needed?

in src/core_write.cpp:None in 480ec93fbb outdated

  46 | +    entry.pushKV("txid", tx.GetHash().GetHex());
  47 | +    entry.pushKV("version", tx.nVersion);
  48 | +    entry.pushKV("locktime", (int64_t)tx.nLockTime);
  49 | +
  50 | +    UniValue vin(UniValue::VARR);
  51 | +    BOOST_FOREACH(const CTxIn& txin, tx.vin) {

theuni commented at 8:28 PM on June 23, 2014:

why not just use a const iterator here?

jgarzik commented at 9:23 PM on July 27, 2014:

A fair point, though I am deferring this until later. ToTxUnix() is a line-by-line translation of TxToJSON() logic, and matching that becomes useful for later unification.

If you wanted to submit a PR fixing TxToJSON(), that would helpful.

in src/core_write.cpp:None in 480ec93fbb outdated

  64 | +        vin.push(in);
  65 | +    }
  66 | +    entry.pushKV("vin", vin);
  67 | +
  68 | +    UniValue vout(UniValue::VARR);
  69 | +    for (unsigned int i = 0; i < tx.vout.size(); i++) {

theuni commented at 8:29 PM on June 23, 2014:

const iterator would make this a good bit easier to read too, imo. std::distance to get the old 'i' value.

laanwj commented at 8:02 AM on July 23, 2014:

@theuni I don't agree std::distance is better, it's much less known, and has O(N) complexity for any container that's not random access which would be terrible in a loop. It's a pity that C++ doesn't have Python's enumerate() that returns both the object and index.

theuni commented at 4:18 PM on July 23, 2014:

@laanwj Sure, it was just a quick suggestion to help with readability.

For the sake of discussion: this iterator is random access, so it'd be a constant complexity. Imo it's good practice to use iterators when looping through a container and touching each (non-primitive) element. The life-cycle and validity semantics are more clear to the reader, and the const_iterator would make its purpose explicit. I'll agree that std::distance is a bit obscure, so a separate incrementor would've probably been a better suggestion here.

jgarzik commented at 3:32 AM on June 24, 2014: contributor

Rebased for CMutableTransaction, and promoting code movement commits above others for better reviewing and merging.

jgarzik commented at 3:37 PM on June 25, 2014: contributor

Updated for @theuni 's feedback.

For some odd reason, the recent feedback from @sipa cannot be found. I recall the "{" style nits. I think there was something else too?

jgarzik commented at 1:15 AM on June 26, 2014: contributor

Rebased for merge. Added PR dependency #4415

jgarzik commented at 3:06 PM on June 29, 2014: contributor

This has one remaining FIXME blocking wider whole-tree use of UniValue, but that is not a blocker for merging this PR.

I think all feedback from @theuni and @sipa has been addressed. I cannot find the @laanwj feedback; @laanwj can you recall your comments?

laanwj commented at 3:14 PM on June 29, 2014: member

@jgarzik I'd like univalue* in separate directory inside src, as it is an independent library like crypto and the current json. it doesn't need any of the bitcoin objects.

in src/univalue.h:None in 2c757a69ab outdated

  79 | +        return 0;
  80 | +    }
  81 | +
  82 | +    bool getBool() const { return isTrue(); }
  83 | +    bool getArray(std::vector<UniValue>& arr);
  84 | +    bool getObject(std::map<std::string,UniValue>& obj);

sipa commented at 3:34 PM on June 29, 2014:

I'd rather not see big "convert the entire internal structure to some map/vector type" methods. They don't add anything, as the caller (or whoever you're passing the map/vector to) still needs a dependency on UniValue, so they might as well use UniValue's own accessor methods (operator[], ... maybe UniValue needs iterators for the array case).

The only place where they seem actually used is as a type-check, where is* instead of get* could be used.

sipa commented at 3:36 PM on June 29, 2014: member

+1 on moving to a separate source directory.

jgarzik commented at 7:01 PM on July 16, 2014: contributor

Univalue moved into separate source directory univalue/

jgarzik commented at 6:30 PM on July 18, 2014: contributor

nits addressed.

laanwj commented at 10:32 AM on July 23, 2014: member

ACK. I think it's time to merge this. It compiles for Linux, MacOSX and Windows and works in at least Linux and Windows (tested summarily).

We do need unit tests for univalue, as well as tests invoking the new bitcoin-tx utility, but this can be done in a later pull.

laanwj commented at 9:08 AM on July 25, 2014: member

@sipa, @theuni, can we have a last ACK here?

in src/Makefile.am:None in bbf0b9c9b2 outdated

 181 | @@ -177,6 +182,14 @@ crypto_libbitcoin_crypto_a_SOURCES = \
 182 |    crypto/sha1.h \
 183 |    crypto/ripemd160.h
 184 |  
 185 | +# univalue JSON library
 186 | +univalue_libbitcoin_univalue_a_CPPFLAGS = $(BITCOIN_CONFIG_INCLUDES)

theuni commented at 2:38 PM on July 25, 2014:

The config includes shouldn't be needed, since univalue doesn't include bitcoin-config.h.

theuni commented at 2:49 PM on July 25, 2014:

Just tested to be 100% sure, removing it is fine.

theuni commented at 2:40 PM on July 25, 2014: member

Build-side ACK after addressing that last note. The rest of my comments were just nits, so nothing to worry about.

Side-note: Great job adding univalue as a dependency-less lib!

jgarzik commented at 4:24 PM on July 26, 2014: contributor

Updated for @theuni comment.

in src/bitcoin-tx.cpp:None in c84972a67e outdated

  65 | +        strUsage += "  locktime=N             " + _("Set TX lock time to N") + "\n";
  66 | +        strUsage += "  nversion=N             " + _("Set TX version to N") + "\n";
  67 | +        strUsage += "  outaddr=VALUE:ADDRESS  " + _("Add address-based output to TX") + "\n";
  68 | +        strUsage += "  outscript=VALUE:SCRIPT " + _("Add raw script output to TX") + "\n";
  69 | +        strUsage += "  sign=SIGHASH-FLAGS     " + _("Add zero or more signatures to transaction") + "\n";
  70 | +        strUsage += "      This command requires JSON registers:\n";

sipa commented at 3:26 PM on July 27, 2014:

What are 'registers' in this context?

jgarzik commented at 3:28 PM on July 27, 2014:

A (string key=JSON value) pair, whose name and JSON object are settable from the command line.

It is used to provide named parameters to some of the more complicated commands.

You can load register contents (the JSON value) from a file, or command line argv[] string.

sipa commented at 3:32 PM on July 27, 2014:

Never mind!

in src/bitcoin-tx.cpp:None in c84972a67e outdated

 203 | +    tx.vout.push_back(txout);
 204 | +}
 205 | +
 206 | +static void MutateTxAddOutScript(CMutableTransaction& tx, const string& strInput)
 207 | +{
 208 | +    // separate VALUE:ADDRESS in string

sipa commented at 3:30 PM on July 27, 2014:

VALUE:SCRIPT, I presume?

in src/bitcoin-tx.cpp:None in c84972a67e outdated

  73 | +        strUsage += "      See signrawtransaction docs for format of sighash flags, JSON objects.\n";
  74 | +        strUsage += "\n";
  75 | +        fprintf(stdout, "%s", strUsage.c_str());
  76 | +
  77 | +        strUsage = _("Register Commands:") + "\n";
  78 | +        strUsage += "  load.NAME=FILE         " + _("Load JSON file FILE into register NAME") + "\n";

sipa commented at 3:33 PM on July 27, 2014:

Suggest other syntax: load=file:FILE. "file:X" is never valid JSON anyway.

sipa commented at 3:41 PM on July 27, 2014:

I mean: NAME=file:FILE.

sipa commented at 4:02 PM on July 27, 2014:

After IRC discussion: propose: set=NAME:VALUE and load=NAME:FILE.

in src/core_io.h:None in c84972a67e outdated

  16 | +
  17 | +// core_write.cpp
  18 | +extern std::string EncodeHexTx(const CTransaction& tx);
  19 | +extern void ScriptPubKeyToUniv(const CScript& scriptPubKey,
  20 | +                        UniValue& out, bool fIncludeHex);
  21 | +extern void TxToUniv(const CTransaction& tx, const uint256 hashBlock, UniValue& entry);

sipa commented at 3:36 PM on July 27, 2014:

If you pass that uint256 by reference, you can drop the #include "uint256.h" and turn it into a forward declaration.

in src/bitcoin-tx.cpp:None in c84972a67e outdated

 345 | +        if (!prevtxsObj.getArray(prevTxs))
 346 | +            throw runtime_error("prevtxs not an array");
 347 | +
 348 | +        for (unsigned int previdx = 0; previdx < prevTxs.size(); previdx++) {
 349 | +            map<string,UniValue> prevOut;
 350 | +            if (!prevTxs[previdx].getObject(prevOut))

sipa commented at 3:39 PM on July 27, 2014:

Why do you need to copy the entire internal map? You can use prevTxs[previdx] directly as a map (with its operator[]).

in src/bitcoin-tx.cpp:None in c84972a67e outdated

 322 | +    fGivenKeys = true;
 323 | +
 324 | +    vector<UniValue> keys;
 325 | +    if (!keysObj.getArray(keys))
 326 | +        throw runtime_error("privatekeys not an array");
 327 | +    for (unsigned int kidx = 0; kidx < keys.size(); kidx++) {

sipa commented at 3:54 PM on July 27, 2014:

You can use keysObj directly here without copying to keys.

in src/bitcoin-tx.cpp:None in c84972a67e outdated

 340 | +    if (!registers.count("prevtxs"))
 341 | +        throw runtime_error("prevtxs register variable must be set.");
 342 | +    UniValue prevtxsObj = registers["privatekeys"];
 343 | +    {
 344 | +        vector<UniValue> prevTxs;
 345 | +        if (!prevtxsObj.getArray(prevTxs))

sipa commented at 3:55 PM on July 27, 2014:

Same here: no need to copy.

in src/univalue/univalue.h:None in c84972a67e outdated

  56 | +
  57 | +    enum VType getType() const { return typ; }
  58 | +    std::string getValStr() const { return val; }
  59 | +    bool empty() const { return (values.size() == 0); }
  60 | +
  61 | +    size_t size() const {

sipa commented at 3:57 PM on July 27, 2014:

This method has a very ambiguous meaning, being totally different for objects/arrays and strings/numbers. Do you use it anywhere for anything but checking emptyness? Perhaps just a bool empty() const method instead?

jgarzik commented at 5:07 PM on July 27, 2014:

Yes, it is used to examine string size and also to examine array | values array size for further iteration.

Agree that ambiguity is an issue. The scope could be narrowed to arrays+objects, and renamed to arraySize() or somesuch.

sipa commented at 6:53 PM on July 27, 2014:

.size() and .count() perhaps?

in src/univalue/univalue.h:None in c84972a67e outdated

  81 | +
  82 | +    bool getBool() const { return isTrue(); }
  83 | +    bool getArray(std::vector<UniValue>& arr);
  84 | +    bool getObject(std::map<std::string,UniValue>& obj);
  85 | +    bool checkObject(const std::map<std::string,UniValue::VType>& memberTypes);
  86 | +    UniValue operator[](const std::string& key) const;

sipa commented at 4:00 PM on July 27, 2014:

It would be more efficient to return a const UniValue& reference (yes, I want to avoid copying entire structures).

If you want the returned reference to remain valid under array addition, you'll need a std::vector of UniValue* rather than std::vector of UniValue to back the array implementation.

jgarzik commented at 9:41 PM on July 27, 2014:

Agree it is more efficient, though at the moment it is a convention to return a null object to indicate failure, rather than throwing an exception or something else. That seems a useful convention as any cascading, second order failures are more likely to fail-safe.

Would need to clean up that convention, to turn this thing into a reference.

sipa commented at 11:22 PM on July 27, 2014:

No you don't need to. You can have one static const UniValue() object, and return a reference to that in case of invalid/nonexting argument.

in src/bitcoin-tx.cpp:None in c84972a67e outdated

 379 | +            coins.vout[nOut].nValue = 0; // we don't know the actual output value
 380 | +            view.SetCoins(txid, coins);
 381 | +
 382 | +            // if redeemScript given and not using the local wallet (private keys
 383 | +            // given), add redeemScript to the tempKeystore so it can be signed:
 384 | +            if (fGivenKeys && scriptPubKey.IsPayToScriptHash() &&

sipa commented at 4:21 PM on July 27, 2014:

Perhaps redeemscripts can go into a register too?

jgarzik commented at 9:43 PM on July 27, 2014: contributor

@sipa all concerns should be addressed/responded-to

sipa commented at 11:21 PM on July 27, 2014: member

You're still returning a full copy in operator[] :)

jgarzik commented at 11:24 PM on July 27, 2014: contributor

"or responded to" the operator[] returns a null UniValue which is used usefully in several places in the API.

I agree RE full copy, but fixing requires rethinking the entire class's error handling strategy.

sipa commented at 11:26 PM on July 27, 2014: member

Sorry, I missed that comment. I hate github's per-line commenting.

See my reply there.

sipa commented at 11:49 PM on July 27, 2014: member

It seems you're ignoring DecodeHexTx's return value, and bitcoin-tx accepts incorrectly encoded transactions (and turns the, into... something).

in src/bitcoin-tx.cpp:None in d362e9e2d0 outdated

 395 | +                coins.vout.resize(nOut+1);
 396 | +            coins.vout[nOut].scriptPubKey = scriptPubKey;
 397 | +            coins.vout[nOut].nValue = 0; // we don't know the actual output value
 398 | +            view.SetCoins(txid, coins);
 399 | +
 400 | +            // if redeemScript given and not using the local wallet (private keys

sipa commented at 11:51 PM on July 27, 2014:

This comment doesn't really apply here :)

sipa commented at 12:00 AM on July 28, 2014: member

Mildly tested ACK (but please add at least some warning for invalidly encoded transactions).

jgarzik commented at 4:24 AM on July 28, 2014: contributor

All feedback addressed.

sipa commented at 1:56 PM on July 28, 2014: member

ACK.

PS: the getArray and getObject methods are unused now. Care to remove them?

jgarzik commented at 2:22 PM on July 28, 2014: contributor

@sipa I'll promise not to forget your request to delete them. :) They are used in the off-github "convert tree to univalue" branch. I need to update that to make them go away.

sipa commented at 9:38 PM on July 28, 2014: member

There's --enable-daemon, --enable-cli, --enable-gui and --enable-tests. No --enable-txtool?

jgarzik commented at 9:57 PM on July 28, 2014: contributor

@sipa A fair question. @laanwj or @theuni asked same on IRC. Most distros build everything then pick out what they need. Many projects do not bother with --enable-just-this-one-tool, or find that such configure script features have a user count approaching 1.

It is needless complexity. I would prefer to (a) not add --enable-txtool unless a user really needs it, and (b) consider removing the --enable-just-this-one-tool commands.

The main user build choice vis a vis build time & libs is GUI or not. Everything else quickly reaches a point of diminishing returns.

laanwj commented at 7:39 AM on July 29, 2014: member

BTW; I suppose it should be discouraged to pass private keys on the command line. However, the only alternative at the moment is reading them from a 'register' file. How to handle this securely, maybe an option to read an object from stdin?

laanwj commented at 7:44 AM on July 29, 2014: member

Eh, at least keep the option to enable/disable building the GUI! I'm also quite sure that some people have come to rely on options for enabling/disabling the other executables so if it's not too much trouble I'd prefer to keep them.

sipa commented at 12:20 PM on July 29, 2014: member

Squash some commits? :)

Consolidate CTransaction hex encode/decode into core_io.h, core_{read,write}.cpp ae775b5b31

Move ParseScript() helper, becoming accessible outside src/test/ b2aeaa7939

core_read's ParseScript(): minor cleanups

- use .empty() rather than .size() == 0
- use a const_iterator rather than BOOST_FOREACH
- validate iterators before creating a string from them

2a5840096f

bitcoin-cli, rpcrawtransaction: harmonize "{" styling 3ce7e669e3

Add "bitcoin-tx" command line utility and supporting modules.

This is a simple utility that provides command line manipulation of
a hex-encoded TX. The utility takes a hex string on the command line
as input, performs zero or more mutations, and outputs a hex string
to standard output.

This utility is also an intentional exercise of the "bitcoin library"
concept. It is designed to require minimal libraries, and works
entirely without need for any RPC or P2P communication.

See "bitcoin-tx --help" for command and options summary.

cbe39a3852

jgarzik commented at 3:18 PM on July 29, 2014: contributor

Rebased and commits collapsed.

BitcoinPullTester commented at 3:34 PM on July 29, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4332_cbe39a38526a6c17619d02cc697b80ebfd57203b/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

jgarzik added the label Feature on Jul 31, 2014

jgarzik added the label Improvement on Jul 31, 2014

jgarzik added the label Priority Medium on Jul 31, 2014

jgarzik added this to the milestone 0.10.0 on Jul 31, 2014

jgarzik merged this on Jul 31, 2014

jgarzik closed this on Jul 31, 2014

jgarzik deleted the branch on Aug 24, 2014

MarcoFalke locked this on Sep 8, 2021