Armory - Discussion Thread - page 227.

etotheipi

legendary

Activity: 1428

Merit: 1093

Core Armory Developer

Quote from: SgtSpike on January 07, 2012, 02:55:20 AM

Dude, sounds good to me!
Question though: Can you not generate offline transactions directly on the machine where the private keys reside?

Armory tries to detect three things: satoshi-client connection, general internet connection, and blk0001.dat. If any of those of are missing, you will get this dialog, or something similar. However, I have not yet detected the case that all three are available but the blockchain is not sync'd. That's a case I will let fall through the cracks, for now (though, when I get full networking, I will be able to detect that we're not sync'd yet, and disable transactions).

Quote from: ShadowOfHarbringer on January 07, 2012, 07:45:25 AM

I am sure you are aware that in the current implementation of the mainline Bitcoin client, transaction fees are forced even when not necessary (which is the reason i created the fork of mine).
So, does your client allow no-fee transactions ?

Forced-fees are the default, but they are possible to disable if you manually edit the settings file. I did this to make it possible to disable them, but I don't want it to look like I encourage it in anyway. I don't want anyone complaining when their tx gets stuck (yes, I know it's rare, but I'm still responsible).

ShadowOfHarbringer

legendary

Activity: 1470

Merit: 1006

Bringing Legendary Har® to you since 1952

@etotheipi

I am sure you are aware that in the current implementation of the mainline Bitcoin client, transaction fees are forced even when not necessary (which is the reason i created the fork of mine).

So, does your client allow no-fee transactions ?

FreeMoney

legendary

Activity: 1246

Merit: 1016

Strength in numbers

Quote from: SgtSpike on January 07, 2012, 02:55:20 AM

Dude, sounds good to me!

Question though: Can you not generate offline transactions directly on the machine where the private keys reside?

Maybe not if the offline computer doesn't have an up to date blockchain to know which of it's addresses are funded? Maybe you could tell it, but that wouldn't be much simpler than doing as it's being done.

Total guess here, please confirm.

SgtSpike

legendary

Activity: 1400

Merit: 1005

Quote from: etotheipi on January 07, 2012, 01:37:19 AM

More teasers! And I need some recommendations... I figured you folks would appreciate seeing more screenshots, anyway

This dialog is loaded whenever you try to send money from a watching-only wallet. The goal is to explain how the user can execute the offline transaction without needing external advice (yes, some users will still not totally understand it, but that's why this is only available in the "Advanced" usermode).

I have spent a lot of time trying to fix the layout and the wording, and I still feel like it could be better. What better way than to get feedback from the potential users!

-Eto

P.S. - Sorry I haven't gotten around to the Windows build instructions ... I want to get the pre-alpha code out as soon as possible before battling Windows issues. Building in Windows really is a PITA. See post #63 if you want to compile and run in Ubuntu, which is only a few shell commands.

Dude, sounds good to me!

Question though: Can you not generate offline transactions directly on the machine where the private keys reside?

etotheipi

legendary

Activity: 1428

Merit: 1093

Core Armory Developer

More teasers! And I need some recommendations... I figured you folks would appreciate seeing more screenshots, anyway

This dialog is loaded whenever you try to send money from a watching-only wallet. The goal is to explain how the user can execute the offline transaction without needing external advice (yes, some users will still not totally understand it, but that's why this is only available in the "Advanced" usermode).

I have spent a lot of time trying to fix the layout and the wording, and I still feel like it could be better. What better way than to get feedback from the potential users!

-Eto

P.S. - Sorry I haven't gotten around to the Windows build instructions ... I want to get the pre-alpha code out as soon as possible before battling Windows issues. Building in Windows really is a PITA. See post #63 if you want to compile and run in Ubuntu, which is only a few shell commands.

LightRider

legendary

Activity: 1500

Merit: 1022

I advocate the Zeitgeist Movement & Venus Project.

Good work, I'll be testing this!

etotheipi

legendary

Activity: 1428

Merit: 1093

Core Armory Developer

Quote from: Mike Hearn on January 05, 2012, 04:17:14 AM

No, it's I who should apologise. You're right, there is handling of re-orgs in there. I was taking a quick look over the code before heading to sleep last night (I try for every new client that is open source) and stopped reading at that point. It's a little confusing that the comments refer to parts of this code saying it's not implemented (this happens in addNewBlock too), when they are, but that doesn't excuse me not checking more carefully.

I sent you a donation by way of apology.

I've got a few more questions - do you run through the re-org tests manually? I also did a quick look for code that verifies the wallet contents after re-orgs in unittest.py, but couldn't find it.

When a spend is created and is waiting to confirm, then a re-org invalidates the tx that was spent, does that show up in the ui? Where is it handled?

Thanks for the apology. It sounds like I need to go through the code and clean up some old comments. I like to leave myself "TODO"s" and random comments about what isn't done yet, and then forget to remove them.

The reorg test is actually in the C++ unittests: cppForSwig/BlockUtilsTest.cpp, and I believe the .dat files have been checked in as part of the test, too. It's not part of the UI yet but you just reminded me that I need to make it part of the UI. Perhaps check the isValid_ flag for a LedgerEntry and pop up some kind of message in the rare event it goes false. I think the user should be notified that they just became a victim...

Mike Hearn

legendary

Activity: 1526

Merit: 1134

No, it's I who should apologise. You're right, there is handling of re-orgs in there. I was taking a quick look over the code before heading to sleep last night (I try for every new client that is open source) and stopped reading at that point. It's a little confusing that the comments refer to parts of this code saying it's not implemented (this happens in addNewBlock too), when they are, but that doesn't excuse me not checking more carefully.

I sent you a donation by way of apology.

I've got a few more questions - do you run through the re-org tests manually? I also did a quick look for code that verifies the wallet contents after re-orgs in unittest.py, but couldn't find it.

When a spend is created and is waiting to confirm, then a re-org invalidates the tx that was spent, does that show up in the ui? Where is it handled?

btcinstant

full member

Activity: 180

Merit: 100

Brillant just brillant!

jothan

full member

Activity: 184

Merit: 100

Feel the coffee, be the coffee.

Quote from: etotheipi on January 04, 2012, 02:06:34 PM

Quote from: Mike Hearn on January 04, 2012, 01:28:08 PM

mmap does need the address space, so for a large chain you'd need a 64 bit binary/cpu. However it is not platform specific. Every platform has mmap or an equivalent. On Windows it's called MapViewOfFileEx:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa366763(v=vs.85).aspx

It is platform specific if I have to use a completely different implementation for each platform, unless they have identical interface calls (which I doubt). Specifically, I'll have to do preprocessor branching to compile one set of code in Windows and another branch for Linux (and maybe yet another for OSX). The question is how much code has to be branched to make it work? My guess is that it's a lot...

As for address space comment, am I correct that mmap doesn't help me? It sounds like if the blockchain is 2.0 GB, I have to reserve 2.0 GB of RAM to mmap it -- therefore I'm not really benefitting any folks who only have 1-2 GB of total system RAM. Perhaps someone with more mmap experience than me can explain if there's anything I can get out of it, and how much different the code is going to look between platforms.

You would need a thin layer of glue code, it should not be that bad. On 32 bits, you may need to separate the block chain in 256MiB or 512MiB chunks (and limit the number of chunks loaded) to map at a time to avoid the 1-3 GiB mmap limit.

As for efficiency, it all depends on your access patterns. The memory system will conspire to keep often-used pages in RAM to speed up their access. Using madvise or similar on other platforms may help before doing a full scan. If there is no such thing on Windows, the glue code can be an empty function. I also understand your need to keep things simple at this point. mmaping on 64 bits should, except for the initial setup, be identical to your current scheme.

I also forgot to say good job !
Looks wonderful so far !

Keep it up ! I'll fork some coins over sometime soon (I'm waiting for a money transfer).

Cheers !

SgtSpike

legendary

Activity: 1400

Merit: 1005

Quote from: Mike Hearn on January 04, 2012, 04:16:22 PM

It's probably just a few lines of code. They have nearly identical behaviors so it's really not a big deal.

mmap maps regions of address space to files. When you access them the OS faults the underlying file region into memory if necessary. Therefore you do not need 2GB of RAM to load a 2GB file. You do however need 2GB of address space. On a 64 bit system (regardless of how much physical RAM you have) this is not an issue. On 32 bit you're not going to achieve that because most OS' fragment the address space somewhat and indeed typically reserve the top one or two gigs of address space for the kernel.

Well, that is a good point, but then, who is running a 64-bit OS with less than 4GB of ram? Most people only utilize a 64-bit implementation of an OS so they can address 4GB+ of RAM. I bet the percentage of users using less than 4GB of ram on a 64-bit OS would be extremely small, which would render the work of implementing virtual memory into this client rather useless.

If it's not a big deal to implement, go for it, but if it's going to take away from development of other features, or bug fixed, etc, I would first do a market study to find out exactly how many people NEED a virtual memory implementation. You know, a cost vs benefit analysis and all of that.

etotheipi

legendary

Activity: 1428

Merit: 1093

Core Armory Developer

Quote from: Mike Hearn on January 04, 2012, 04:16:22 PM

...
Yes, you probably need to do something important. Re-org handling is a chore but a Bitcoin implementation that doesn't do it is incorrect and will mysteriously fail, suffer security problems or corrupt its internal data structures at some random future point in time.

I hope you address this in future and provide a convincing set of unit tests. And in general tackle correctness before cool features.

You should look closer at the code, because you completely misunderstood what you read. I already ran an exhaustive blockchain re-organization unit-test and posted them on the forums a couple months ago. Not only does my code gracefully handle re-orgs and double spends, but I even packaged and documented it, and put it on the forums for everyone else.

I spent a 1-2 weeks painstakingly creating that unit test and stepping through the debugger hundreds of times to make sure it works perfectly. I'm not pleased that you accuse me of "not focusing on correctness" when I have 2500 lines of unit testing code in my project. Those comments you referenced were for additional things that I might want to do on a re-org, outside of updating the blockchain data structures. At the moment, there isn't anything else.

(To see the reorg-handling code, do a search for the variable txJustInvalidated_, and the function "reassessAfterReorg" in BlockUtils.cpp)

BitcoinBug

full member

Activity: 196

Merit: 100

Looks like a hell lot of work.
Congrats!

Mike Hearn

legendary

Activity: 1526

Merit: 1134

It's probably just a few lines of code. They have nearly identical behaviors so it's really not a big deal.

mmap maps regions of address space to files. When you access them the OS faults the underlying file region into memory if necessary. Therefore you do not need 2GB of RAM to load a 2GB file. You do however need 2GB of address space. On a 64 bit system (regardless of how much physical RAM you have) this is not an issue. On 32 bit you're not going to achieve that because most OS' fragment the address space somewhat and indeed typically reserve the top one or two gigs of address space for the kernel.

Having taken a look at the code, I would be careful with recommending users put any significant money into this app. It's great that you implemented useful features, but I see the following:

Code:

         if(!blockIsNewTop)
         {
            cout << "Block data did not extend the main chain!" << endl;
            // TODO:  Not sure if there's anything we need to do if this block
            //        didn't extend the main chain.
         }
   
         if(blockchainReorg)
         {
            cout << "This block forced a reorg!  (and we're going to do nothing...)" << endl;
            //TODO:  do something important (besides seg-faulting)
         }

Yes, you probably need to do something important. Re-org handling is a chore but a Bitcoin implementation that doesn't do it is incorrect and will mysteriously fail, suffer security problems or corrupt its internal data structures at some random future point in time.

I hope you address this in future and provide a convincing set of unit tests. And in general tackle correctness before cool features.

Libreyseco

newbie

Activity: 6

Merit: 0

It's great, I like it. Thank you for this increidble Bitcoin client.

etotheipi

legendary

Activity: 1428

Merit: 1093

Core Armory Developer

Quote from: Mike Hearn on January 04, 2012, 01:28:08 PM

mmap does need the address space, so for a large chain you'd need a 64 bit binary/cpu. However it is not platform specific. Every platform has mmap or an equivalent. On Windows it's called MapViewOfFileEx:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa366763(v=vs.85).aspx

It is platform specific if I have to use a completely different implementation for each platform, unless they have identical interface calls (which I doubt). Specifically, I'll have to do preprocessor branching to compile one set of code in Windows and another branch for Linux (and maybe yet another for OSX). The question is how much code has to be branched to make it work? My guess is that it's a lot...

As for address space comment, am I correct that mmap doesn't help me? It sounds like if the blockchain is 2.0 GB, I have to reserve 2.0 GB of RAM to mmap it -- therefore I'm not really benefitting any folks who only have 1-2 GB of total system RAM. Perhaps someone with more mmap experience than me can explain if there's anything I can get out of it, and how much different the code is going to look between platforms.

Mike Hearn

legendary

Activity: 1526

Merit: 1134

mmap does need the address space, so for a large chain you'd need a 64 bit binary/cpu. However it is not platform specific. Every platform has mmap or an equivalent. On Windows it's called MapViewOfFileEx:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa366763(v=vs.85).aspx

etotheipi

legendary

Activity: 1428

Merit: 1093

Core Armory Developer

Quote from: gnar1ta$ on January 04, 2012, 12:49:17 PM

Looks great, unfortunately I use OSX. Undecided

Once I get official build-instructions out there for Windows and Linux (and actually make the first release), I'm sure someone will figure out how to port it to OSX. I've tried hard to use platform-independent code, so hopefully it won't be too difficult.

gnar1ta$

donator

Activity: 798

Merit: 500

Looks great, unfortunately I use OSX. Undecided

P4man

hero member

Activity: 518

Merit: 500

Looking very very good indeed, congrats. Subbed and ill test it tomorrow.

Topic: Armory - Discussion Thread - page 227. (Read 521855 times)