Re: [Bitcoin-development] 0.7 merge recommendations/status
On Sat, Mar 31, 2012 at 01:16:56PM +0200, Michael Grønager wrote: > If you are interested, I could push libcoin to bitcoin (e.g. bitcoin/libcoin) > and then you could build bitcoind bitcoin-qt on libcoin. > > libcoin solved most of the problems you list below. And if you worry about > the copyright/license I am also willing to change that to make it fit. Thanks for that - without a license change it would not be possible to merge anything. > libcoin have no global thread mutexes and and there is no blocking of the > main thread due to rpc methods (except for a sendto), further, e.g. a > reorganize only locks the main thread for a split second while the final > commit is done. Yes, I like its design and refactorings a lot, but at the same time it's very large change to accept at once. In particular, I'm not entirely convinced yet about its thread-safety. For example, acceptblock is a public method, but it seems (i may be missing something) to grab no lock at all until setBestBlock or reorganize is called. Is it impossible to call acceptBlock twice simultaneously? One may start with a bestblockindex value that gets modified a split second later by a simultaneous call. It may be the case that there are indeed no possibilities for this to happen because of things I'm missing, but although I'm a big fan of well-encapsulated locks and the use of reader-writer locks, it's hard to verify whether you use them enough. My suggestion would be: make each publicly accessible method of BlockChain grab either a reader lock (if it's a const function) or an upgradable lock, and take a writer lock in each method that actually performs changes. > The libcoin rpc supports keep_alive and pipelining, runs in its own thread > (but can also run in the same thread as the node) and uses async operation. > Ipv6 is easy to implement in libcoin as the CAddress/Endpoint class is > implemented as a subclass of boost::endpoint, only thing holding back is > deciding on an ipv6 format on IRC, and, I then I would really like to reverse > the order of the last 12 bytes in the address db (they are opposite to boost). Not sure what you mean: the serialized combination of the 32-bit IPv4 address and 12 bytes padding in CAddress are exactly a bsd socket library in6_addr in network byte order. In 0.6.0, CAddress derives from CNetAddr, which encapsulates these 16 bytes. -- Pieter -- This SF email is sponsosred by: Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd2d-msazure ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] 0.7 merge recommendations/status
If you are interested, I could push libcoin to bitcoin (e.g. bitcoin/libcoin) and then you could build bitcoind bitcoin-qt on libcoin. libcoin solved most of the problems you list below. And if you worry about the copyright/license I am also willing to change that to make it fit. libcoin have no global thread mutexes and and there is no blocking of the main thread due to rpc methods (except for a sendto), further, e.g. a reorganize only locks the main thread for a split second while the final commit is done. The libcoin rpc supports keep_alive and pipelining, runs in its own thread (but can also run in the same thread as the node) and uses async operation. Ipv6 is easy to implement in libcoin as the CAddress/Endpoint class is implemented as a subclass of boost::endpoint, only thing holding back is deciding on an ipv6 format on IRC, and, I then I would really like to reverse the order of the last 12 bytes in the address db (they are opposite to boost). Cheers, Michael On 31/03/2012, at 12:54, Pieter Wuille wrote: > On Sat, Mar 31, 2012 at 12:03:17AM -0400, Luke-Jr wrote: >> NOTE: I've been piecing this together for about a week now, and intended to >> update it when 0.6.0 final was released, but with the timing of it, I just >> won't get the time to update for a while, so here is my last draft... > > Nice summary, thanks. > >> It seems to me, there is potentially enough ready to merge into 0.7 to start >> the RC process right away if someone wants to... except that the first merge >> will probably require rebasing everything else ;) > > I think that's right - for several reasons, the time between 0.5 and 0.6 was > over 4 months. I prefer more frequent releases, as it slows down development > this way. > >> For similar reasons as CBlockStore, I feel multithreaded JSON-RPC with keep- >> alive support (#568) should be merged sooner rather than later. It's long >> overdue for bitcoind having had a lot of testing, and pretty much required >> for >> any sort of high-volume bitcoind usage (such as solo mining). Some other >> optimizations by Joel such as the optimized ToHex function (#562) and >> FastGetWork (#565) have also had plenty of testing; all combined, these >> optimizations more than double the performance of JSON-RPC. >> Details: https://github.com/bitcoin/bitcoin/pull/565#issuecomment-3269334 > > I'd rather see a decent encapsulation of wallet and blockchain data structures > that allow us to make their mutexes private, and let only the code from the > respective mutex take locks in it when necessary. That will automatically > lead to multithreaded RPC, but in a safe way, without needing guesswork about > which two calls may or may not be called simultaneously. > > Of course, that requires a lot more work, but at some point that will be > needed > anyway, imho. > >> Pieter's getalltransactions (#841) and my getblock_full (#886) provide what >> is >> needed to completely replace Jeff's old dumpblock call with bitcoind's new >> getblock. He also put together a -loadblock option (#883) which has proven >> quite handy for development, and -walletupgrade (#974) seems like a good >> idea. > > I've used loadblocks often in my personal branches. At least on Linux it seems > to work fine. The data scanning code is mostly Cish though, and there may be > more preferrable to use boost or generic C++ solutions. > >> Finally, I don't know the status of Pieter's IPv6 support, but I hope it >> will >> be ready for 0.7. Right now all I see submitted for this is support for >> multiple local IPs (#829) though. > > I've already had a fully functional IPv6 node based on 0.3.24. Most of the > changes > there have since been incorported in netbase (#735), and because of a risk > for DoS'es > based on the much larger number of addresses an attacker could have under his > control, > addrman (#787) was necessary before IPv6 could be fully implemented. So, the > technical > part of supporting IPv6 seems mostly finished - right now, it's mostly just > removing > some (!IsIPv4()) checks and adding listen/connect code that is > IPv6-compatible. > I'll do a pullreq for that soon. > > There are a few other issues, though. For example: how will relaying work: > will IPv4 > nodes relay IPv6 addresses? If not, the IPv6 bitcoin network will be > completely > separate from the IPv4 one, though both may overlap in some points. The > opposite is > also possible: allowing all nodes to relay IPv6 addresses, but only use them > in case > an IPv6-compatible interface is detected. Any opinions about this? > > Something else was suggested by Jeff: what if a node accidentally connects to > itself? > As we're moving towards multiple local addresses with IPv6, the chances for > this > become larger. Finally, there are always extra risks involved, as we could > unknowingly > be opening DoS or others vulnerabilities. > > Finally, supporting IPv6 in a somewhat general way would pave the wa
Re: [Bitcoin-development] 0.7 merge recommendations/status
On Sat, Mar 31, 2012 at 12:54:02PM +0200, Pieter Wuille wrote: > Something else was suggested by Jeff: what if a node accidentally connects to > itself? > As we're moving towards multiple local addresses with IPv6, the chances for > this > become larger. Finally, there are always extra risks involved, as we could > unknowingly > be opening DoS or others vulnerabilities. My mistake: I mean two nodes connecting twice to eachother. There is already protection against a node connecting to itself. -- Pieter -- This SF email is sponsosred by: Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd2d-msazure ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] 0.7 merge recommendations/status
On Sat, Mar 31, 2012 at 12:03:17AM -0400, Luke-Jr wrote: > NOTE: I've been piecing this together for about a week now, and intended to > update it when 0.6.0 final was released, but with the timing of it, I just > won't get the time to update for a while, so here is my last draft... Nice summary, thanks. > It seems to me, there is potentially enough ready to merge into 0.7 to start > the RC process right away if someone wants to... except that the first merge > will probably require rebasing everything else ;) I think that's right - for several reasons, the time between 0.5 and 0.6 was over 4 months. I prefer more frequent releases, as it slows down development this way. > For similar reasons as CBlockStore, I feel multithreaded JSON-RPC with keep- > alive support (#568) should be merged sooner rather than later. It's long > overdue for bitcoind having had a lot of testing, and pretty much required > for > any sort of high-volume bitcoind usage (such as solo mining). Some other > optimizations by Joel such as the optimized ToHex function (#562) and > FastGetWork (#565) have also had plenty of testing; all combined, these > optimizations more than double the performance of JSON-RPC. > Details: https://github.com/bitcoin/bitcoin/pull/565#issuecomment-3269334 I'd rather see a decent encapsulation of wallet and blockchain data structures that allow us to make their mutexes private, and let only the code from the respective mutex take locks in it when necessary. That will automatically lead to multithreaded RPC, but in a safe way, without needing guesswork about which two calls may or may not be called simultaneously. Of course, that requires a lot more work, but at some point that will be needed anyway, imho. > Pieter's getalltransactions (#841) and my getblock_full (#886) provide what > is > needed to completely replace Jeff's old dumpblock call with bitcoind's new > getblock. He also put together a -loadblock option (#883) which has proven > quite handy for development, and -walletupgrade (#974) seems like a good idea. I've used loadblocks often in my personal branches. At least on Linux it seems to work fine. The data scanning code is mostly Cish though, and there may be more preferrable to use boost or generic C++ solutions. > Finally, I don't know the status of Pieter's IPv6 support, but I hope it will > be ready for 0.7. Right now all I see submitted for this is support for > multiple local IPs (#829) though. I've already had a fully functional IPv6 node based on 0.3.24. Most of the changes there have since been incorported in netbase (#735), and because of a risk for DoS'es based on the much larger number of addresses an attacker could have under his control, addrman (#787) was necessary before IPv6 could be fully implemented. So, the technical part of supporting IPv6 seems mostly finished - right now, it's mostly just removing some (!IsIPv4()) checks and adding listen/connect code that is IPv6-compatible. I'll do a pullreq for that soon. There are a few other issues, though. For example: how will relaying work: will IPv4 nodes relay IPv6 addresses? If not, the IPv6 bitcoin network will be completely separate from the IPv4 one, though both may overlap in some points. The opposite is also possible: allowing all nodes to relay IPv6 addresses, but only use them in case an IPv6-compatible interface is detected. Any opinions about this? Something else was suggested by Jeff: what if a node accidentally connects to itself? As we're moving towards multiple local addresses with IPv6, the chances for this become larger. Finally, there are always extra risks involved, as we could unknowingly be opening DoS or others vulnerabilities. Finally, supporting IPv6 in a somewhat general way would pave the way for bitcoin functioning for example as a Tor or I2P hidden service, by using onioncat-like tor-encoded-in-IPv6 addresses. This way, two bitcoin nodes could connect to eachother without the need for passing any exit node. -- Pieter -- This SF email is sponsosred by: Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd2d-msazure ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] 0.7 merge recommendations/status
Thanks for the summary! On Sat, Mar 31, 2012 at 6:03 AM, Luke-Jr wrote: > It seems to me, there is potentially enough ready to merge into 0.7 to > start > the RC process right away if someone wants to... except that the first > merge > will probably require rebasing everything else ;) > Yes, we have a lot of changes waiting already. > Next up are some changes already ACK'd for 0.7: Hearn's "pong" message > (#932) > and Wladimir's Visual C++ 2010 fixes (#949). getmemorypool BIP > standardization > (#936) is also ACK'd, but it might be good to wait until later in the merge > window considering its low impact and high potential for change as the BIP > gets closer to Accepted status. > Agreed. > > any sort of high-volume bitcoind usage (such as solo mining). Some other > optimizations by Joel such as the optimized ToHex function (#562) and > See my comments there; I'm all for optimizing the ToHex function, but I prefer that he optimizes the current ToHex function not add yet another one with an incompatible interface. (we have the same problem with Error/Debug/"Log to console" functions, too many of them and sometimes it's unclear what the difference is) > Scott has a pull request for Bitcoin-Qt to behave more like other close-to- > systray applications by toggling the hide/show action (#855). He's also > contributed a patch to show miners' immature balances on the overview > screen > (#837; it leaves only a blank space for non-miners). Nils, on the other > hand, > has been working with a UI designer to totally remodel Bitcoin-Qt. > I also have some UI code changes ready, for example one to use notification from the bitcoin core when the address book/transactions changed, instead of a timer. Will submit pull requests soon. Coderrr has rebased his Coin Control features (#415) to the latest version. > These seem to be popular, so should probably be merged as soon as it's > had proper review. > Agreed. It is very popular and should certainly be merged. And it has seen quite some testing already. Though this will take some time to review, as it is quite a large change. > Finally, I don't know the status of Pieter's IPv6 support, but I hope it > will > be ready for 0.7. Right now all I see submitted for this is support for > multiple local IPs (#829) though. > > IPv6 support would be nice, but I don't think a milestone of 0.7 is realistic. Such a change to the network code will require extensive testing. Who has access to IPv6 and can help testing? Wladimir -- This SF email is sponsosred by: Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd2d-msazure___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
