public inbox for bitcoindev@googlegroups.com
 help / color / mirror / Atom feed
* [Bitcoin-development] 0.7 merge recommendations/status
@ 2012-03-31  4:03 Luke-Jr
  2012-03-31  7:56 ` Wladimir
  2012-03-31 10:54 ` Pieter Wuille
  0 siblings, 2 replies; 6+ messages in thread
From: Luke-Jr @ 2012-03-31  4:03 UTC (permalink / raw)
  To: bitcoin-development

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...

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 ;)

My first recommendation is to merge Matt's CBlockStore (#771). It's mostly a 
major code cleanup, but it still needs a lot of post-merge testing. The sooner 
it gets in the master branch, the more testing of unexpected cases that it 
will get before final. Also, Matt's been working hard to keep rebasing it 
throughout the 0.6 merge window, which is very difficult since it conflicts 
with pretty much every other change. As one of the parties responsible for 
those other changes, I vote to get the big conflict over with and rebase all 
the simpler stuff afterward.

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.

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

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.

Under the hood, Chris has some neat refactoring of the coin selection 
algorithm (#905, #898), and I haven't had any problems using it in next-test 
for a few weeks now. Michael has contributed a patch to get the standard 
reopen-log-files-on-SIGHUP (#917). Matt noticed the protocol documentation on 
the wiki and BitcoinJ both expect the 'getheaders' message to return at most 
only 2000 headers, so recommends we enforce that in the core (#951). Philip 
has a trivial flip to the backslashes in debug.log for Windows (#971). Some 
p2pool miners put up a bounty for a JSON-RPC call to customize fee 
requirements (#989) that would help make Bitcoin more decentralized.

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.

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.

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'd like to see Coinbaser (#719) finally get merged, but since it seems nobody 
is using bitcoind for mining anymore, I guess there isn't a real need. I don't 
plan to rebase this anymore unless someone gives it a "I'll merge it" sign.

Luke



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bitcoin-development] 0.7 merge recommendations/status
  2012-03-31  4:03 [Bitcoin-development] 0.7 merge recommendations/status Luke-Jr
@ 2012-03-31  7:56 ` Wladimir
  2012-03-31 10:54 ` Pieter Wuille
  1 sibling, 0 replies; 6+ messages in thread
From: Wladimir @ 2012-03-31  7:56 UTC (permalink / raw)
  To: Luke-Jr; +Cc: bitcoin-development

[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]

Thanks for the summary!

On Sat, Mar 31, 2012 at 6:03 AM, Luke-Jr <luke@dashjr•org> 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

[-- Attachment #2: Type: text/html, Size: 3703 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bitcoin-development] 0.7 merge recommendations/status
  2012-03-31  4:03 [Bitcoin-development] 0.7 merge recommendations/status Luke-Jr
  2012-03-31  7:56 ` Wladimir
@ 2012-03-31 10:54 ` Pieter Wuille
  2012-03-31 11:08   ` Pieter Wuille
  2012-03-31 11:16   ` Michael Grønager
  1 sibling, 2 replies; 6+ messages in thread
From: Pieter Wuille @ 2012-03-31 10:54 UTC (permalink / raw)
  To: Luke-Jr; +Cc: bitcoin-development

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bitcoin-development] 0.7 merge recommendations/status
  2012-03-31 10:54 ` Pieter Wuille
@ 2012-03-31 11:08   ` Pieter Wuille
  2012-03-31 11:16   ` Michael Grønager
  1 sibling, 0 replies; 6+ messages in thread
From: Pieter Wuille @ 2012-03-31 11:08 UTC (permalink / raw)
  To: Luke-Jr; +Cc: bitcoin-development

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bitcoin-development] 0.7 merge recommendations/status
  2012-03-31 10:54 ` Pieter Wuille
  2012-03-31 11:08   ` Pieter Wuille
@ 2012-03-31 11:16   ` Michael Grønager
  2012-03-31 12:28     ` Pieter Wuille
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Grønager @ 2012-03-31 11:16 UTC (permalink / raw)
  To: Pieter Wuille; +Cc: bitcoin-development

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 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

Michael Gronager, PhD
Director, Ceptacle
Jens Juels Gade 33
2100 Copenhagen E
Mobile: +45 31 45 14 01
E-mail: gronager@ceptacle•com
Web: http://www.ceptacle.com/




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bitcoin-development] 0.7 merge recommendations/status
  2012-03-31 11:16   ` Michael Grønager
@ 2012-03-31 12:28     ` Pieter Wuille
  0 siblings, 0 replies; 6+ messages in thread
From: Pieter Wuille @ 2012-03-31 12:28 UTC (permalink / raw)
  To: Michael Grønager; +Cc: bitcoin-development

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-03-31 12:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-31  4:03 [Bitcoin-development] 0.7 merge recommendations/status Luke-Jr
2012-03-31  7:56 ` Wladimir
2012-03-31 10:54 ` Pieter Wuille
2012-03-31 11:08   ` Pieter Wuille
2012-03-31 11:16   ` Michael Grønager
2012-03-31 12:28     ` Pieter Wuille

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox