Fixpoint

Previously on Fixpoint, I introduced my implementation of the getrawtransaction command and reviewed an existing one for sendrawtransaction. There were some more and less serious problems and stylistic considerations outstanding. To recap, for the "get":

1. Not a proper vpatch.
2. Loose parsing, ignoring trailing non-hex or excess characters in the txid argument.
3. Directly accessing the mapTransactions object without acquiring the cs_mapTransactions lock.
4. Unnecessarily exposing the mapTransactions object globally.

And for the "send", (with some points not previously noted, indicated by *, for my teeth grow sharper with the passage of time):

5. Needing a regrind to build on other work from the intervening years.
6. Redundant inclusion of CDataStream constructor defaults SER_NETWORK and VERSION.
7. Loose parsing, ignoring excess data in the hex argument after the first decoded transaction.
8. Refusal to re-broadcast transactions already in the node's memory pool.
9. Returning -5 for already-seen transactions, a code otherwise used only for invalid arguments or inexistant objects. *
10. New TransactionSeen function, clumsy in both usage and implementation.
11. Supposed "missing inputs" condition that can't occur and wouldn't distinguish error codes if it did.
12. Application of anti-spam checks by way of calling AcceptToMemoryPool with fCheckInputs=true.
13. No reason provided for some rejection cases. *
14. "txHash" breaking the conventional Hungarian notation, suggesting it's a CTransaction instance. *

Now meet bitcoin_rawtx_get_send.vpatch.

It addresses points 1, 3, 4, 5, 6, 8, 9, 10, 11, and 14. The risky exposure of mapTransactions is replaced with specific accessor functions that take care of locking. Each of these also turns out to simplify other code in main.cpp: always a nice sign that you're onto a good interface.

#2 I've discussed previously and don't see as a problem with this patch.

#7 I've let slide; please speak up if you think it's important.

#12 I've preserved deliberately, on the theory that it's better to reject transactions likely to be rejected by peers so the sender has a chance to determine the cause and fix it. This strikes me as a weak argument though; perhaps better would be to have a way to force it if necessary.

#13 is tricky because AcceptToMemoryPool is a monster of a function that could fail for a number of reasons, and doesn't make that reason available to the caller. It does at least write to the debug log so I've made the error message provide that referral.

The two previous patches added up to +99/-2 lines; the new unified one to +85/-10.

Further, I've now made the whole Real Bitcoin patch set and known seals available on my own reference code shelves.⁽ⁱ⁾ I went through them all afresh to identify those on which I was comfortable adding my own signature; for the rest I used my "jfw_unchecked" subordinate key (both keys are available through my contact page).

On that shelf you may also notice a novel bitcoin_system_compiler.vpatch. It amounts to a full overhaul of the build system and will be the subject of an upcoming article.

1. At some point I intend to put a formal, efficient, GNU bloatware free sync mechanism in place. For now, the best I can suggest to grab the whole collection in one shot is:

   wget --mirror --no-parent http://fixpoint.welshcomputing.com/v/

   [^]