[DEV] Fix automatic unit tests Feathercoin 0.9.6 [Completed 29th Jan 2017]

AcidD

Hi All, @wrapper @wellenreiter @Lizhi @ghostlander

After many moons I’ve decided to give fixing the tests another go.

I’ve gotten to the point where if i build the tests, this is my output:

acids-Mac:test acid$ make
Generated data/script_valid.json.h
Generated data/base58_keys_valid.json.h
Generated data/sig_canonical.json.h
Generated data/sig_noncanonical.json.h
Generated data/base58_encode_decode.json.h
Generated data/base58_keys_invalid.json.h
Generated data/script_invalid.json.h
Generated data/tx_invalid.json.h
Generated data/tx_valid.json.h
Generated data/sighash.json.h
Generated data/alertTests.raw.h
/Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
  CXX      test_bitcoin-allocator_tests.o
  CXX      test_bitcoin-base32_tests.o
  CXX      test_bitcoin-base58_tests.o
  CXX      test_bitcoin-base64_tests.o
  CXX      test_bitcoin-bignum_tests.o
  CXX      test_bitcoin-canonical_tests.o
  CXX      test_bitcoin-checkblock_tests.o
  CXX      test_bitcoin-Checkpoints_tests.o
  CXX      test_bitcoin-compress_tests.o
  CXX      test_bitcoin-DoS_tests.o
  CXX      test_bitcoin-getarg_tests.o
  CXX      test_bitcoin-key_tests.o
  CXX      test_bitcoin-main_tests.o
  CXX      test_bitcoin-mruset_tests.o
  CXX      test_bitcoin-multisig_tests.o
  CXX      test_bitcoin-netbase_tests.o
  CXX      test_bitcoin-pmt_tests.o
  CXX      test_bitcoin-rpc_tests.o
  CXX      test_bitcoin-script_P2SH_tests.o
  CXX      test_bitcoin-script_tests.o
  CXX      test_bitcoin-serialize_tests.o
  CXX      test_bitcoin-sigopcount_tests.o
  CXX      test_bitcoin-test_bitcoin.o
  CXX      test_bitcoin-transaction_tests.o
  CXX      test_bitcoin-uint256_tests.o
  CXX      test_bitcoin-util_tests.o
  CXX      test_bitcoin-scriptnum_tests.o
  CXX      test_bitcoin-sighash_tests.o
  CXX      test_bitcoin-accounting_tests.o
  CXX      test_bitcoin-wallet_tests.o
  CXX      test_bitcoin-rpc_wallet_tests.o
  CXXLD    test_bitcoin

and when I run the test program

 acids-Mac:test acid$ ./test_bitcoin
Running 99 test cases...
unknown location:0: fatal error: in "base58_tests/base58_keys_valid_gen": key_error: CKey::SetSecret() : secret must be 32 bytes
base58_tests.cpp:246: last checkpoint
key_tests.cpp:87: error: in "key_tests/key_test1": check key1C.IsCompressed() == true has failed
key_tests.cpp:92: error: in "key_tests/key_test1": check key2C.IsCompressed() == true has failed
key_tests.cpp:101: error: in "key_tests/key_test1": check addr1C.Get() == CTxDestination(pubkey1C.GetID()) has failed
key_tests.cpp:102: error: in "key_tests/key_test1": check addr2C.Get() == CTxDestination(pubkey2C.GetID()) has failed
main_tests.cpp:21: error: in "main_tests/subsidy_limit_test": check nSum == 2099999997690000ULL has failed
transaction_tests.cpp:278: error: in "transaction_tests/test_IsStandard": check !IsStandardTx(t, reason) has failed

*** 7 failures are detected in the test module "Bitcoin Test Suite"
acids-Mac:test acid$

AcidD

Anyone who wants to help can clone this
https://github.com/aciddude/Feathercoin/tree/0.9.6-TESTS

It’s the latest Feathercoin/0.9.6 branch plus my initial test fixes.

You can see a comparison of the branches here:
https://github.com/FeatherCoin/Feathercoin/compare/0.9.6...aciddude:0.9.6-TESTS?expand=1

AcidD

I’ve done a few more fixes

acids-Mac:test acid$ ./test_bitcoin
Running 99 test cases...
unknown location:0: fatal error: in "base58_tests/base58_keys_valid_gen": key_error: CKey::SetSecret() : secret must be 32 bytes
base58_tests.cpp:246: last checkpoint
main_tests.cpp:21: error: in "main_tests/subsidy_limit_test": check nSum == 8399999990760000ULL has failed
transaction_tests.cpp:278: error: in "transaction_tests/test_IsStandard": check !IsStandardTx(t, reason) has failed

*** 3 failures are detected in the test module "Bitcoin Test Suite"
acids-Mac:test acid$

Only 3 failures left!!

wrapper

This is the test we are looking at :

Feathercoin/src/test/main_tests.cpp

#include “core.h”
#include “main.h”

#include <boost/test/unit_test.hpp>

BOOST_AUTO_TEST_SUITE(main_tests)

BOOST_AUTO_TEST_CASE(subsidy_limit_test)
{
uint64_t nSum = 0;
for (int nHeight = 0; nHeight < 14000000; nHeight += 1000) {
uint64_t nSubsidy = GetBlockValue(nHeight, 0);
BOOST_CHECK(nSubsidy <= 200 * COIN);
nSum += nSubsidy * 1000;
BOOST_CHECK(MoneyRange(nSum));
}
BOOST_CHECK(nSum == 2099999997690000ULL);
}

BOOST_AUTO_TEST_SUITE_END()

From src/util.h Coin appears to be static const int64_t COIN = 100000000; or is set else where as the dust coin value.

~~Someone~~ The original Bitcoin test said 50 aand has been converted to l FTC 200 block reward

wrapper

The FTC reward changed at 306960 so it should be something like this (which fails)

BOOST_AUTO_TEST_CASE(subsidy_limit_test)
{
uint64_t nSum = 0;
for (int nHeight = 0; nHeight < 306960; nHeight += 1000) {
uint64_t nSubsidy = GetBlockValue(nHeight, 0);
BOOST_CHECK(nSubsidy <= 200 * COIN);
nSum += nSubsidy * 1000;
BOOST_CHECK(MoneyRange(nSum));
}
BOOST_CHECK(nSum == 33600000000000ULL);
}

wrapper

From : Article of Controlled supply Bitcoin : https://en.bitcoin.it/wiki/Controlled_supply

Note: The number of bitcoins are presented in a floating point format. However, these values are based on the number of satoshi per block originally in integer format to prevent compounding error.

In block 124724, user midnightmagic solo mined a block which caused one less Satoshi to be created than would otherwise have come into existence. Therefore, all calculations from this block onwards must now, to be accurate, include this underpay in total Bitcoins in existence.

AcidD

This morning, I’ve managed to fix a few more of the errors.

This last one is stumping me.

Running 99 test cases...
unknown location:0: fatal error: in "base58_tests/base58_keys_valid_gen": key_error: CKey::SetSecret() : secret must be 32 bytes
base58_tests.cpp:246: last checkpoint

*** 1 failure is detected in the test module "Bitcoin Test Suite"

AcidD

This is the code that’s breaking:

The Bitcoin original test

{
            bool isCompressed = find_value(metadata, "isCompressed").get_bool();
            CKey key;
            key.Set(exp_payload.begin(), exp_payload.end(), isCompressed);
            assert(key.IsValid());
            CBitcoinSecret secret;
            secret.SetKey(key);
            BOOST_CHECK_MESSAGE(secret.ToString() == exp_base58string, "result mismatch: " + strTest);
        }

Trying to copy what Lizhi has done else where in the code

       {
            bool isCompressed = find_value(metadata, "isCompressed").get_bool();
            CSecret secret1;
            CKey key;
            key.SetSecret(secret1, isCompressed);
            assert(key.IsValid());
            CBitcoinSecret secret;
            secret.SetSecret(secret1, isCompressed);
            BOOST_CHECK_MESSAGE(secret.ToString() == exp_base58string, "result mismatch: " + strTest);
        }

The above code builds but then still fails with


acids-Mac:test acid$ ./test_bitcoin
Running 99 test cases...
unknown location:0: fatal error: in "base58_tests/base58_keys_valid_gen": key_error: CKey::SetSecret() : secret must be 32 bytes
base58_tests.cpp:247: last checkpoint

*** 1 failure is detected in the test module "Bitcoin Test Suite"

AcidD

one bit of good news is that if I disable that one test. all the tests pass with no errors detected. This is on FTC 0.9.6 ! This makes me happy.

it’s now “98 test cases” since I disabled base58_tests/base58_keys_valid_gen

acids-Mac:test acid$ ./test_bitcoin
Running 98 test cases...

*** No errors detected
acids-Mac:test acid$

wrapper

Having most of the test now build for the FTC 0.9.6 build , gives more confidence in the build. Especially as it is a bug fix / refinement / document updated build of Feathercoin version 0.9.3

We can move this knowledge on to get the auto test up to date for 0.11. Great work and kudos to Aciddude.

Aciddude and I will still be working on it (base58_tests/base58_keys_valid_gen) , if we can get a fix it can be backported from a 0.9.7-dev (further fixes to 0.9 series)

@Lizhi or @Ghostlander might have any idea why the test is failing that might help us fix it.

Heres the FTC source code with tests - https://github.com/aciddude/Feathercoin/tree/0.9.6-TESTS

Test build guide for Ubuntu 16.04 :

http://forum.feathercoin.com/topic/8556/guide-build-feathercoind-feathercoin-qt-v0-9-3-1-on-ubuntu-16-04-lts/15

Feathercoin Docs :Ubuntu build instructions Readme /docs :

https://github.com/FeatherCoin/Feathercoin/tree/0.9.6/doc

Wellenreiter

@aciddude

Great work and man thanks for your efforts.

So we have a fix for github issue #104, too :)

Also many thanks to @wrapper for hist support

AcidD

Hi All,

I’ve fixed the final testcase
BOOST_AUTO_TEST_CASE(base58_keys_valid_gen)

We now have 99 unit tests passing for the 0.9.6 release.

wrapper

Thanks @Aciddude well done keeping at i t…

AcidD

Test built and working on:

MacOS Sierra

Ubuntu 16.04 LTS

wrapper

This is the last test, the test crawls through values of nHeight, or " simulated" blocknumber and accumulates nSum, up to a maximum value ( 23919999). At each 1000 “block” it does a calculation , based on COIN, a constant and the miners subsidy.

then, this cumulated value is “boost compared” to what it should be, weirdly, +ULL.

Notes :

copyright needs updating, to the FTC corrected Test files.
“Tests” needs testing to see if what happens at subsidy halving

Two separate tests were created to account for the subsidy change over on block 204639, when the block time was reduced from 2.5 mins to 1 minute, and subsidy changed from 200 to 80 …

Feathercoin/src/test/main_tests.cpp

#include “core.h”
#include “main.h”

#include <boost/test/unit_test.hpp>
BOOST_AUTO_TEST_SUITE(main_tests)
BOOST_AUTO_TEST_CASE(subsidy_limit_test)
{
uint64_t nSum = 0;
for (int nHeight = 0 ; nHeight < 204638; nHeight += 1000) {
uint64_t nSubsidy = GetBlockValue(nHeight, 0);
BOOST_CHECK(nSubsidy <= 200 * COIN);
nSum += nSubsidy * 1000;
BOOST_CHECK(MoneyRange(nSum));
}
for (int nHeight = 204639 ; nHeight < 23919999; nHeight += 1000) {
uint64_t nSubsidy = GetBlockValue(nHeight, 0);
BOOST_CHECK(nSubsidy <= 80 * COIN);
nSum += nSubsidy * 1000;
BOOST_CHECK(MoneyRange(nSum));
}
BOOST_CHECK_EQUAL(nSum, 33599996093750000ULL);
}
BOOST_AUTO_TEST_SUITE_END()

Wellenreiter

We should enhance the test section to test acceptance of Block version 2 and Block version 4 starting 0.9.6, but I’m not sure if that can be done within a few days. I would like to announce 0.9.6 as new production release before February.

wrapper

@Wellenreiter A test to cover the Block 2 and 4 issue is a very good idea. It should be relatively easy for @Lizhi to do, with his experience of the code and information in the tests thread and commits.

I propose we remove the Block 2 and 4 compatibility unless there is a (auto test suite) test and it passes. I would help create one. Obviously we would also need @Aciddude , it shouldn’t be that difficult …

I have created an issue. https://github.com/FeatherCoin/Feathercoin/issues/151

Wellenreiter

@wrapper said in [DEV] Fix Build Auto Tests Feathercoin 0.9.6 [Completed]:

@Wellenreiter A test to cover the Block 2 and 4 issue is a very good idea. It should be relatively easy for @Lizhi to do, with his experience of the code and information in the tests thread and commits.

I propose we remove the Block 2 and 4 compatibility unless there is a (auto test suite) test and it passes. I would help create one. Obviously we would also need @Aciddude , it shouldn’t be that difficult …

I have created an issue. https://github.com/FeatherCoin/Feathercoin/issues/151

I disagree with the removal of the Block 2/Block 4 code. 0.9.6 is planned to be the ‘bridge’ between the old and new block version, so it is essential, that it accepts both block versions.
We can do a manual test in a testnet setup to check for this release and build the automated tests for the next releases. See my comment on Github for that.

https://github.com/FeatherCoin/Feathercoin/issues/151

wrapper

Issue
I am particularly concerned that we will get support issues if we implement the “Bridge” Feathercoin Block 2/Block 4 code forward compatibility, under the current circumstances.

Mode of success
I would agree we could release a Tested version, if there were an official Feathercoin 0.11 version binary out. But current there is a possibility of the current 0.11 development version being applied to mining.

Causes of Failure
People have taken the highest number as the official version of Feathercoin in the past, causing us major problems before.

Symptom of failure
I see this as an accident waiting to happen, so if we go ahead I won’t be able to “do support issues” that arise.

Suggested Way forward

Make sure there are no ways to download a “Unofficial” Feathercoin 0.11 Binary and @Lizhi promises not to deploy test or development versions of Feathercoin 0.11 until is ready for release.

We create a higher version of Feathercoin 0.11 (i.e. 0.11.1 so any rouge binaries are not the highest release number) , which is also a Bridge version and does not try trigger a block 4 upgrade, We can then test that operates on the network ready for the “Official 0.11 release” which would allow the block change trigger…

When we change to Official 0.11 the block 4 change can be well publicized auto fork /0.8 upgrade essential fork.

Wellenreiter

The Bridge functionality is tested already, and I’d say we tested it the hard way. Last spring we had Lizhi’s 0.11.2 code starting to create Block v4 which was rejected by the 0.8.7 and 0.9.3 versions, which in turn continued to creat Block version 2, that was rejected by the 0.11.2.
The result was the fork of the BC we faced.
Ghostlander impleneted a patch for 0.8.7 into 0.8.7.3 to accept also block version 4 and Lizhi did the similar thing to 0.9.3.2.
The BC converted after these patched releases where rolled out.
I think, that was the ultimative proof, that the code is working.

Additionally Lizhi modifed the 0.11.X code to also accept Block V2 blocks.

I think we are safe here.

Now we release 0.9.6 and officially name it a bridge release to motivate users to migrate a bit faster. ;)

Regarding Lizhi’s development versions, I currently have a close look on Github and check that the production flag is set to false for any new merge/push that is made to the code.
This generates a warning in the gui and the logs that the release is non-production and should not be used for mining.

I think we hardly can do more, as anybody can fork the code, do any modification and compile/run a dangerous client.

We can’t avoid that by implementing more complex administrative measures. As long as the big majority of nodes is using the ‘official’ code, the infrastructure should cover any errors injected by a single or small number of misbehaving clients. That is part of the distributed BC.