General questions arising when adding Namecoin support to Armory

[picobit]

Thanks for the help! I am able to receive and send testnet Namecoins with Armory! I haven't tried testing it with mainnet yet.

I did run into a small bug where, after sending a transaction, the pop-up in the taskbar and the transactions tab on the main screen say I only sent 0.005, when I really sent 1 plus a 0.005 fee. I am on the dev branch, but haven't brought it up to date for a few days. So maybe it is a known bug that is already fixed. I'll do more testing to determine whether this is Namecoin specific or not and see if it is fixed with an up-to-date dev branch. Once there is a confirmation the transactions tab correctly lists the amount as 1.005. Also, when double clicking on the transaction, the window that pops up displays the correct amount, even if there is not yet a confirmation.

I need to disable P2SH and multisig for Namecoin for now. I might need to tweak the fee calculation code too. Then I'll try to write that unit test.

If you sent it to yourself (i.e. another address in the same wallet) then the list on the transaction tab will only show the fee, since that is the net amount leaving the account.  The symbol next to the transaction will be different - there is one for outgoing, one for incoming, and one for send to self.

[josephbisch]

Thanks for the help! I am able to receive and send testnet Namecoins with Armory! I haven't tried testing it with mainnet yet.

I did run into a small bug where, after sending a transaction, the pop-up in the taskbar and the transactions tab on the main screen say I only sent 0.005, when I really sent 1 plus a 0.005 fee. I am on the dev branch, but haven't brought it up to date for a few days. So maybe it is a known bug that is already fixed. I'll do more testing to determine whether this is Namecoin specific or not and see if it is fixed with an up-to-date dev branch. Once there is a confirmation the transactions tab correctly lists the amount as 1.005. Also, when double clicking on the transaction, the window that pops up displays the correct amount, even if there is not yet a confirmation.

I need to disable P2SH and multisig for Namecoin for now. I might need to tweak the fee calculation code too. Then I'll try to write that unit test.

If you sent it to yourself (i.e. another address in the same wallet) then the list on the transaction tab will only show the fee, since that is the net amount leaving the account.  The symbol next to the transaction will be different - there is one for outgoing, one for incoming, and one for send to self.

I sent it from an address in Armory to an address in Namecoin-qt.

[josephbisch]

Was this line of this commit supposed to be removed? Because I am getting "AttributeError: 'ArmoryMainWindow' object has no attribute 'lastSDMState'" when trying to start ArmoryQt.py with an up-to-date dev branch. I'm sure this is probably already known, or some error on my part. Adding the line back allows ArmoryQt.py to launch, and it appears to work.

Edit: Nope, ArmoryQt.py does launch, but it gets stuck at the block syncing stage. It won't proceed to create the Armory db.

[goatpig]

the ZC value issue was fixed, but the dev branch is in shambles currently.

[josephbisch]

Thanks, it's good to know that the dev branch is in a broken state and it's not just me.

I pushed what I have so far to my GitHub fork.

I think I disabled multisig for Namecoin, but I haven't tested it due to the state of dev. Looking through the code, it appears that leaving P2SHBYTE as an empty string is enough to disable P2SH for Namecoin (because when doing comparisons, an empty string is never going to == a byte), so I think I have disabled that too.

I looked at the fee calculation code and there are three functions for calculating fees. It looks like one of the more complex parts of the Namecoin integration. I guess I can leave the multisig calculation one alone for now, since Namecoin doesn't currently support multisig. That leaves calcMinSuggestedFees and calcMinSuggestedFeesNew and it looks like I need to modify both for Namecoin.

I'll watch the dev branch to see when ArmoryQt.py is runnable again. In the meantime I'll finish up the Namecoin support including that unit test you asked for.

[CircusPeanut]

Was this line of this commit supposed to be removed? Because I am getting "AttributeError: 'ArmoryMainWindow' object has no attribute 'lastSDMState'" when trying to start ArmoryQt.py with an up-to-date dev branch. I'm sure this is probably already known, or some error on my part. Adding the line back allows ArmoryQt.py to launch, and it appears to work.

Edit: Nope, ArmoryQt.py does launch, but it gets stuck at the block syncing stage. It won't proceed to create the Armory db.

Thanks for the heads up.

I put that line back so it won't crash there.

There still seem to be issues with running BitcoinD from Armory on Testnet. I usually uncheck "Let Armory start Bitcoin..." in the Settings menu, and start BitcoinD myself when I'm working with Testnet.

[josephbisch]

Was this line of this commit supposed to be removed? Because I am getting "AttributeError: 'ArmoryMainWindow' object has no attribute 'lastSDMState'" when trying to start ArmoryQt.py with an up-to-date dev branch. I'm sure this is probably already known, or some error on my part. Adding the line back allows ArmoryQt.py to launch, and it appears to work.

Edit: Nope, ArmoryQt.py does launch, but it gets stuck at the block syncing stage. It won't proceed to create the Armory db.

Thanks for the heads up.

I put that line back so it won't crash there.

There still seem to be issues with running BitcoinD from Armory on Testnet. I usually uncheck "Let Armory start Bitcoin..." in the Settings menu, and start BitcoinD myself when I'm working with Testnet.

Thanks, I tried putting the line back myself and had issues with both Namecoin and Bitcoin as my edit said, so I will try it with starting namecoind and bitcoind myself.

Edit: Starting namecoind myself helped with the issues. I'm amazed at the speed of rebuilding and rescanning the db with the recent updates. It happened almost instantaneously (granted I was on Namecoin testnet, so there was a lot less data than Bitcoin mainnet, but still).

[josephbisch]

If anyone reading this wants to help speed up the process of getting Namecoin support into Armory, they can help by testing Namecoin in Armory. If you do, make sure you only use testnet Namecoins, because this hasn't been thoroughly tested. To do so:

1) Git clone this repository
2) git checkout namecoin
3) make
4) python ArmoryQt.py --namecoin-testnet

Those instructions work for me on Linux. They may vary for other OS.

You might need to go to options, uncheck "let Armory run bitcoind in the background", and then manually run namecoind before starting Armory (run namecoind with the -testnet flag). I needed to do so.

If you do test Namecoin integration, please let me know, even if you don't experience any errors.

One thing to note is that I haven't changed the text, so Armory will still say Bitcoin and BTC even though you provided the --namecoin-testnet flag to it. But if you look at the addresses being generated, you will see that they are testnet addresses.

[josephbisch]

About that unit test for NMC, should I put it in CppBlockUtilsTests.cpp or make a separate file for it? I think I'll try to work on it tomorrow.

I think the unit test, text changes (replacing Bitcoin with Namecoin), and making it so Namecoin is not hardcoded in the C++ are the only things stopping this from being ready to be reviewed by Armory devs. But then again I haven't received any indication that anyone else besides me has been testing this code, so it is possible there are bugs I haven't discovered. I'm unsure about the fee calculation code, though in my tests it appeared to work correctly.

I might have to hand the code off with Namecoin hardcoded in the C++, because I am really lost on how to access the config data. I should be ready sometime this week for the code to be reviewed!

[josephbisch]

I pushed an update that replaces Bitcoin/bitcoins/BTC with a function (getCoinText()) that returns the correct name of coin in the correct format. I should have replaced most instances with the function except for a few cases. Those are:

References to Casascius coins. There are no NMC Casascius coins, so I guess I will just get rid of that text when NMC is activated. Or I could leave it as an example of the type of keys it is referring to.

The torrent download (like in the settings panel). I don't think there is a torrent for NMC, so I will completely disable that.

The "bitcoin:" links (I realized I still haven't changed that for NMC (namecoin:), so I still need to do that).

Let me know if you discover another location where you don't see NMC or Namecoin or namecoins when you are testing with Namecoin enabled.

[goatpig]

About that unit test for NMC, should I put it in CppBlockUtilsTests.cpp or make a separate file for it? I think I'll try to work on it tomorrow.

I think the unit test, text changes (replacing Bitcoin with Namecoin), and making it so Namecoin is not hardcoded in the C++ are the only things stopping this from being ready to be reviewed by Armory devs. But then again I haven't received any indication that anyone else besides me has been testing this code, so it is possible there are bugs I haven't discovered. I'm unsure about the fee calculation code, though in my tests it appeared to work correctly.

I might have to hand the code off with Namecoin hardcoded in the C++, because I am really lost on how to access the config data. I should be ready sometime this week for the code to be reviewed!

You should obviously fork the repo and keep your NMC unit test in a dedicated file to cover your changes so it doesn't get modified after merging in changes on our end. That way you have a static unit test to throw at our changes to make sure your modifications remain consistent pull to pull.

dev should be back to a functional state since last Sunday afternoon. It may go under for a while again though. Depending on how large NMC mainnet is you may run in the issue I'm currently working on, so let me know and I'll get you to bypass it for now.

Edit: Starting namecoind myself helped with the issues. I'm amazed at the speed of rebuilding and rescanning the db with the recent updates. It happened almost instantaneously (granted I was on Namecoin testnet, so there was a lot less data than Bitcoin mainnet, but still).

As CircusPeanut said, testnet has never worked with auto managed bitcoind. There's some snafu with the path resolving on the Python side I think, but this is such low priority we have left this issue unattended for over a year now.

As for the speed, I got rid of some scan concurrency recently, so that got a bit slower but in a few commits I'll crank the speed up again. For reference I get Bitcoin testnet supernode scanned in less than 3 minutes.

[josephbisch]

I already forked the repo (available here fyi), so I'll add a new file to it for the unit test under cppForSwig/gtest/.

I'm not sure what issue you are running into, but I just tried Namecoin mainnet and I am stuck synchronizing at 22%. It has been at most a few days since I last synced Namecoin mainnet, so it shouldn't be as low as 22%. I've been mostly testing Namecoin testnet anyway, so if it is your issue, then it isn't a big deal to me. Namecoin mainnet is just under a couple of gigs, fyi.

I think I am going to make a separate thread for testing Namecoin integration. I'm not getting anyone to test it and this thread's poor choice of title probably has to do something with that. I'll continue posting my questions/comments about development here.

[josephbisch]

Is there an example test that inits the BDM with a Bitcoin block (so that I can adapt it to Namecoin)? Because I wrote a test that calls unserializeFullBlock and then checks a lot of the data in the block, but I don't think that is what you had in mind, because it has nothing to do with the BDM.

You can see what tests I have so far.

[goatpig]

You want to pass this test for BDM init operations: https://github.com/etotheipi/BitcoinArmory/blob/dev/cppForSwig/gtest/CppBlockUtilsTests.cpp#L6794

[josephbisch]

I pushed a bunch more changes, including more text changes and support for "namecoin:" URIs. Also I disabled the torrent for Namecoin.

Now I think the only things left are the unit test, support for Namecoin in the built-in downloader, and support for Namecoin in armoryd.

[josephbisch]

I copied that test over to my NamecoinTests.cpp file (removing the lockbox portion, since NMC doesn't support that currently). I also copied the necessary functions to run that test. When running the test, I get output indicating that it is running, but it appears to be stuck running that test.

I used createTestChain.py, modifying it to load the Namecoin genesis block instead of the Bitcoin one, setting fakeblock to True, and using the Namecoin magic bytes.

The line "TheBDM.doInitialSyncOnLoad(nullProgress);" seems to be the one that is hanging. Does fakeblock need to be False when running createTestChain.py for this test?

[goatpig]

I copied that test over to my NamecoinTests.cpp file (removing the lockbox portion, since NMC doesn't support that currently). I also copied the necessary functions to run that test. When running the test, I get output indicating that it is running, but it appears to be stuck running that test.

I used createTestChain.py, modifying it to load the Namecoin genesis block instead of the Bitcoin one, setting fakeblock to True, and using the Namecoin magic bytes.

The line "TheBDM.doInitialSyncOnLoad(nullProgress);" seems to be the one that is hanging. Does fakeblock need to be False when running createTestChain.py for this test?

It can be either. We set it to false to skip difficulty (use 0 for each block's nonce) in order to create test chains a lot faster.

The line hanging is the the one initializing the backend. You should set a few breakpoints there and see where it is choking. 

[josephbisch]

I traced the issue to BlockWriteBatcher::applyBlocksToDB with gdb. I should have looked in the test log file initially, because there were errors there relating to BlockWriteBatcher. Specifically there seems to be a problem with the size of the headers. Which I don't understand, because the output of createTestChain.py seems to indicate that the headers are 80 bytes as expected.

Code:

Log file opened at 1416341503: namecoinTestsLog.txt
-INFO  - 1416341503: (BlockUtils.cpp:626) Set blkfile dir: ./blkfiletest
-INFO  - 1416341503: (BlockUtils.cpp:627) Set leveldb dir: ./ldbtestdir
-INFO  - 1416341503: (lmdb_wrapper.cpp:370) Opening databases...
-INFO  - 1416341503: (BlockDataViewer.cpp:37) Registering Wallet wallet1
-INFO  - 1416341513: (BlockUtils.cpp:948) Executing: doInitialSyncOnLoad
-INFO  - 1416341516: (BlockUtils.cpp:1020) Total number of blk*.dat files: 1
-INFO  - 1416341517: (BlockUtils.cpp:1021) Total blockchain bytes: 5,432
-INFO  - 1416341519: (BlockUtils.cpp:1365) Reading headers from db
-INFO  - 1416341519: (BlockUtils.cpp:1378) Found 7 headers in db
-DEBUG - 1416341520: (Blockchain.cpp:165) Organizing chain w/ rebuild
-WARN  - 1416341522: (BlockUtils.cpp:1059) --- Fetching SSH summaries for 6 registered addresses
-INFO  - 1416341524: (BlockUtils.cpp:1072) Left off at file 0, offset 4729
-INFO  - 1416341526: (BlockUtils.cpp:1075) Reading headers and building chain...
-INFO  - 1416341526: (BlockUtils.cpp:1076) Starting at block file 0 offset 4729
-INFO  - 1416341527: (BlockUtils.cpp:1078) Block height 0
-WARN  - 1416341529: (Blockchain.cpp:37) Somehow tried to add header that's already in map
-WARN  - 1416341529: (Blockchain.cpp:38)     Header Hash: 1964e08c87932373561aec082a68b80abf62f57e5a02c4d521ebbd839cf60de8
-DEBUG - 1416341530: (Blockchain.cpp:165) Organizing chain w/ rebuild
-ERROR - 1416341531: (StoredBlockObj.cpp:221) Asked to unserialize a non-80-byte header
-ERROR - 1416341531: (lmdb_wrapper.cpp:1353) Attempting to put uninitialized bare header into DB
-INFO  - 1416341535: (BlockUtils.cpp:1251) Loading block data... file 0 offset 4729
-INFO  - 1416341535: (BlockUtils.cpp:331) Reading raw blocks finished at file 0 offset 5432
-WARN  - 1416341763: (BlockUtils.cpp:810) Scanning from 0 to 0
-ERROR - 1416342155: (lmdb_wrapper.cpp:1199) Block height exceeds DupID lookup table
-ERROR - 1416342155: (BlockWriteBatcher.cpp:1281) No block in DB at height 0
-ERROR - 1416342156: (lmdb_wrapper.cpp:1199) Block height exceeds DupID lookup table
-ERROR - 1416342156: (BlockWriteBatcher.cpp:1281) No block in DB at height 0

Edit: I put the block file through my Python parser and the data being output looks okay to me. These blocks don't even have the extra Namecoin data yet. And the weirdest thing is that this issue only presents itself when running the tests.

[goatpig]

I traced the issue to BlockWriteBatcher::applyBlocksToDB with gdb. I should have looked in the test log file initially, because there were errors there relating to BlockWriteBatcher. Specifically there seems to be a problem with the size of the headers.

This is what I expect is happening (as a peudo traceback):

Code:

BlockWriteBatcher::applyBlocksToDB 
   BlockWriteBatcher::grabBlocksFromDB
      dis->iface_->getStoredHeader
         ... bunch of calls that end up with block deserialization ...
            StoredHeader::unserializeDBValue

               
This method eventually gets to this:

Code:

      // Unserialize the raw header into the SBH object
      brr.get_BinaryData(dataCopy_, HEADER_SIZE);
      BtcUtils::getHash256(dataCopy_, thisHash_);
      numTx_    = brr.get_uint32_t();
      numBytes_ = brr.get_uint32_t();

If you modified HEADER_SIZE, it's going to choke on non NMC blocks.

[josephbisch]

I saw that code. The problem is I don't modify HEADER_SIZE (because the actual 80 byte header is the same for Namecoin). Which is why I find all of this quite odd.