Bitcoin Unlimited Code Changes
gandrewstone merged 1 commit by ptschip from “ptschip:dev_travis_fail” into “BitcoinUnlimited:dev”. During the merge, 1 file was changed for a total of 0 additions and 334 deletions. These changes remove several tests from the “exploit_tests.cpp” file due to an error. Following a previously merged Pull Request which added a series of new tests to the “exploit_tests.cpp” file, an unforeseen error started occurring when the code was deployed to Travis for testing. Travis CI is a service utilized for the testing of code functionality that is hosted on GitHub. Travis is a helpful service as it doesn’t charge any fees for the testing of a project so long as that project is open source. Naturally, bitcoin projects fit well into this open source framework, and as a result, all of the major bitcoin full node clients utilize the services at Travis CI. Developer ptcship had previously committed the aforementioned Pull Request which contained several new tests designed specifically for the initial “Handshake” that occurs between clients regarding an XTHIN block. Due to a currently unknown error, Travis returns an error when the test code is deployed to it. Developer ptschip noted that he is unsure of which of the tests is currently causing the failure, and will need time to review each test again seeking the culprit. In the meantime, this Pull Request removes only the newly merged tests from “exploit_tests.cpp” to allow the code to continue to be tested without the Travis error that is resulting due to their presence. This Pull Request is a temporary measure that will be reversed once the issue in the tests has been resolved.
gandrewstone merged 1 commit by deadalnix from “deadalnix:norbf” into “BitcoinUnlimited:dev”. During the merge, 3 files were changed for a total of 6 additions and 216 deletions. These changes remove several sections of the “Replace by Fee” code that was present in the client. Replace by Fee has always been a hotly contested topic due to the nature of it’s behavior. Replace by Fee allows a user to replace a transaction already in the memory pool of a node with a transaction bearing the same unspent outputs so long as the second transaction bears a higher fee than the first. This behavior meets the criteria of double spending a transaction as the transaction is unconfirmed when this switch occurs, or rather not yet written into a block. The bitcoin Unlimited team has taken a strong stance against the Replace by Fee feature, originally authored by Peter Todd and implemented in bitcoin Core. Developer sickpig of bitcoin Unlimited published a statement clearing outlining the teams hardline stance against the feature. The changes expressed in this sections Pull Request reflect this stance. Author of the Pull deadalnix states in the authors comments that: “Any feature helping double spend has nothing to do in a p2p cash system.” These changes go on to remove the “Replace by Fee” code that was present in the client through backporting from bitcoin Core. Reviewer ftrader commented that while the committed changes were logical, there were still left over code elements present in the client that related to Replace by Fee. On the subject, deadalnix replied: “The leftover are about generating Replce By Fee transactions in the wallet. This removes Replace By Fee from the node code, not the wallet code.” deadalnix also addressed his future intention towards these remnant Replace by Fee code features: “It’ll get removed from the wallet as well, it is completely independent from this.”
gandrewstone merged 3 commits by ftrader from “ftrader-bitcoinunlimited:feature/gtest_parallel_bitcoin” into “BitcoinUnlimited:dev”. During the merge, 3 files were changed for a total of 675 additions and 7 deletions. These changes bring a new tool to the Unlimited client which allows test built in the Boost test framework to run in parallel. The Boost Test framework is a library of interfaces for writing and controlling simple tests that is used primarily in the development of software. The major bitcoin clients all make use of the Boost Test framework in different ways, and bitcoin Unlimited is no exception to this. Boost tests can be extremely in depth if they are programmed as such, and as a result can take a long period of time to run in full. As a solution to this lengthy testing duration, an adaption of a piece of software originally written by Google was made by developer ftrader. This adapted software is designed to run Unit Tests written in C++ within the Boost Framework in parallel with each other allowing for dramatic speed ups during testing. As the code utilizes a command line parameter specific for Boost, this parallel testing tool will only work in tandem with Boost version 1.59 or later. The tool references the clients “test_bitcoin” system’s executable, and queries it for a list of runnable tests. Once queried, the tests it receives will all run in parallel. This tool is also optimized for ongoing performance increases by storing a testing score per unit test which is stored in the the pre-determined file labelled “~/.gtest-parallel-bitcoin-times”. These scores allow the tool to reorder the testing so that the longest tests are always engaged first which drastically helps to reduce lag and run time. In testing, this tool has proven itself capable of reducing the duration of Boost testing exponentially. In reference to this fact, the developer states: “Testing on a desktop machine with sufficient cores shows that the total test execution time when using this parallelization script is reduced to approximately the running time of the slowest unit test.” The changes outlined in this Pull Request mirror changes made to the bitcoin Classic client described a couple of weeks ago.
Bitcoin Classic Code Changes
zander committed “Fix de-serialization bug where AddrMan is corrupted after exception” with EthanHeilman into “bitcoinclassic:master” within 4 files for a total of 150 additions and 1 deletion. These changes are a backport from bitcoin core, and deal with the clients “Addrman” address manager. Addrman is defined as a “Stochastic Address Manager”, or rather a “randomly determined address manager”. The Addrman manager was developed by bitcoin Core developer sipa. The manager stores the address data of different nodes both known and unknown. This serves as countermeasure to a sybil attack where a single localized attacker may try to completely overwhelm a node by taking over all of it’s connections. The changes in this Pull Request deal with a deserialization bug that was occurring which corrupted Addrman after it threw an exception. These changes achieve this resolution in several ways. Developer EthanHeilman describes his commits. First: “CAddrDB modified so that when de-serialization code throws an exception Addrman is reset to a clean state.” Second: “CAddrDB modified to make unit tests possible.” Third: “Regression test created to ensure bug is fixed.” And last: “StartNode modifed to clear adrman if CAddrDB::Read returns an error code.”
zander committed “Remove non-determinism which is breaking net_tests” with EthanHeilman into “bitcoinclassic:master” within 1 file for a total of 9 additions and 0 deletions. These changes also a backport from bitcoin Core and similarly deal with the clients Addrman address manager. As the Addrman manager stores bitcoin node addresses, it also carries the task of sorting them for usage. Up to this point, the manager achieved this non-deterministically. Non-deterministic algorithms are algorithms which can exhibit different behavior per different runs even with the same inputs. This is the inverse behavior of a deterministic algorithm which will always complete in the same number of steps and return the same result. The changes in this Pull Request remove the non-deterministic behavior of the system due to it causing an unexpected error during network testing. This was achieved by simply adding in a simple piece of code forcing determinism in it’s place, which is written as such:
“+ void MakeDeterministic()
Bitcoin Core Code Changes
iaanwj merged 1 commit by jnewbery from “jnewbery:JSON_refactor” into “bitcoin:master”. During the merge, 27 files were changed for a total of 62 additions and 78 deletions. These changes deal with refactoring the code of “TxToJSON()” and “ScriptPubKeyToJSON()” respectively. Developer jnewbery describes the changes of his commit: “This Pull Request removes almost all of the ‘TxToJSON()’ and ‘ScriptPubKeyToJSON()’ code from ‘bitcoin-rpc’ and places it in ‘bitcoin-common’, removing about 80 lines of duplicate code.” He goes on to state: “The only part that can’t be moved into ‘bitcoin-common’ is parsing block contextual information (confirmations and blocktime), which are not available to bitcoin-common code.” This refactoring helps to ensure proper organization of the code specific to these systems making it an easier task to amend and expand in the future. Tjnewbery also states in the comments of his Pull that this Request should be merged alongside another Pull Request that deals with data related to these systems. Following the merging of his referenced Pull Request, jnewbery rebased this Pull Request on top of it.
iaanwj merged 1 of his commits from “laanwj:2017_04_rpc_if_guidelines” into “bitcoin:master”. During the merge, 1 file was changed for a total of 73 additions and 0 deletions. These additions bring a new document outlining the guidelines for introducing and reviewing new Remote Procedure Call interfaces. This document was added to the “Developer Notes” folder, and serves as a point of reference to anyone seeking to submit a commit to the bitcoin Core code base. These interface guidelines are important for several reasons. Overall, developers do not always follow the same “house keeping” procedures, so providing an outline of desired practices helps to order the work being submitted. Also, the document elaborates on the systems and styles that bitcoin Core uses which allows developers to produce code that is compatible with the code base itself. Such documentation is important to order the work being submitted to bitcoin Core for consideration, and is a valuable guide to make available to the public. These changes were merged following some light review by developers kallewoof, jonasschnelli, fanquake, and sipa.
iaanwj merged 2 commits by jnewbery from “jnewbery:remove_func_test_code_duplication” into “bitcoin:master”. During the merge, 12 files were changes for a total of 235 additions and 513 deletions. These changes remove duplicate code that was present in relation to tests. Developer jnewbery describes the presence of these duplicate bodies of code: “Test cases that use bitcoind’s p2p interface use ‘NodeConnCB’ or a subclass to access the p2p interface. A lot of the test cases use a subclass of ‘NodeConnCB’ and implement their own callbacks or helper methods. In many cases those local implementations are duplicates.” As a means of resolving these duplicates, he states: “This Pull Request adds sensible callbacks and helper functions to the base ‘NodeConnCB’ class, and removes the subclass override methods. Some tests still require custom behavior and so need to define or override some of the class methods.” He goes on to elaborate about each individual commit in this Pull. He describes the first commit as: “The first commit adds the sensible default callbacks and helper methods to NodeConnCB.” The second of the two commits he describes as: “The second commit removes any duplicate implementations in the individual testcsases.” As a measure of the efficiency of this Pull Request, jnewbery adds on: “Net code change is ~ minus 300 lines”
iaanwj merged 2 commits by jnewbery from “jnewbery:mempoolpersistenceoption” into “bitcoin:master”. During the merge, 4 files were changed for a total of 101 additions and 4 deletions. These changes deal with “Mempool Persistence” and it’s control. Following a previously merged Pull Request authored by sipa, several reviewers of said Pull requested a command line parameter capable of toggling the Mempool Persistence feature on or off. These changes bring this parameter to the client. The parameter, titled “-persistmempool”, is set to “True” or On by default. If a user were to toggle the parameter to “False” or Off, two thing would occur. First, the client will not load “mempool.dat” during startup. Second, the client will not write “mempool.dat” when it is shutdown. jnewbery also notes that the groundwork laid out by these changes is also a pre-requisite for the merging of another Pull Request as well. Apart from introducing and integrating this new command line parameter, these changes also include tests specifically for Mempool Persistence and the command line parameter itself.
The scaling debate is approaching its breaking point, and now more than ever, it is important for every individual, not just miners and software developers, to do their part. All readers are recommended to download and establish a full node supporting the client of their choice should they have the technological and economical means to do so. Download links for each of the major full node clients:
– Bitcoin Unlimited – (Pro immediate block size increase via hard fork, Emergent Consensus)
– Bitcoin Classic – (In line with the view points of bitcoin Unlimited, Emergent Consensus)
– Bitcoin XT – (In line with the view points of Classic and Unlimited)
– Bitcoin Core – (Block size increase slowly over several years, immediate soft fork w/ Segwit)
If any reader feels daunted, confused, or overwhelmed by this task, please do not hesitate to reach out