[Bitcoin-development] Useful bitcoin patches...

[Bitcoin-development] Useful bitcoin patches...

[Jeff Garzik]

This was posted to IRC:
http://davids.webmaster.com/~davids/bitcoin-3diff.txt

Includes several useful features that all the big pools have been
screaming for...  notably HTTP/1.1 keep-alive support.

-- 
Jeff Garzik
exMULTI, Inc.
jgarzik@exmulti.com

Re: [Bitcoin-development] Useful bitcoin patches...

[Christian Decker]

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

Some appear to be beneficial to everybody.
Multithreading the RPC will certainly speed up quite a few services and I
see no downside in adding it. The same is true for Keep-Alive.

I'm against including the long polling support because incredibly few people
will benefit from it (pool providers) and yet it is included for everyone.

The Hub mode is good, and I would go a step further and optimize the
connection logic for all nodes by default.

Just IMHO

Regards,
Chris


On Fri, Jul 1, 2011 at 5:23 AM, Jeff Garzik <jgarzik@exmulti.com> wrote:

> This was posted to IRC:
> http://davids.webmaster.com/~davids/bitcoin-3diff.txt
>
> Includes several useful features that all the big pools have been
> screaming for...  notably HTTP/1.1 keep-alive support.
>
> --
> Jeff Garzik
> exMULTI, Inc.
> jgarzik@exmulti.com
>
>
> ------------------------------------------------------------------------------
> All of the data generated in your IT infrastructure is seriously valuable.
> Why? It contains a definitive record of application performance, security
> threats, fraudulent activity, and more. Splunk takes this data and makes
> sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-d2d-c2
> _______________________________________________
> Bitcoin-development mailing list
> Bitcoin-development@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bitcoin-development
>

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

Re: [Bitcoin-development] Useful bitcoin patches...

[Gregory Maxwell]

On Fri, Jul 1, 2011 at 12:03 PM, Christian Decker
<decker.christian@gmail.com> wrote:
> Some appear to be beneficial to everybody.
> Multithreading the RPC will certainly speed up quite a few services and I
> see no downside in adding it. The same is true for Keep-Alive.

The multithreaded RPC stuff will need very aggressive testing to make
sure it doesn't expose race conditions elsewhere in the code.

E.g. you don't want to lose change from a send because some txn called
getnewaddress concurrently and there was a bug. So far the
multithreaded RPC patches have pretty much only been run by miners...
who have a different rpc profile than everyone else.

(and the MT RPC that I've been using only multhreaded getwork…)

> The Hub mode is good, and I would go a step further and optimize the
> connection logic for all nodes by default.

Gah. No.

The 'hub mode' is not good. We're already low on sockets network wide,
adding a built in DDOS mode flag to bitcoin that makes nodes
aggressively connect to lots of neighbors is a bad idea. People will
ignorantly enable it thinking they are adding resources to the network
when they are really consuming much much more.

I have a big fast node with a higher connection limit and the flood
fixes and I'm currently seeing 596 inbound connections right now. This
suggests the situation is already a lot worse than the rough numbers
using lfnet connection counts suggested.


Miners, concerned with fast block propagation, should manually addnode
each other. We should fix the addnode logic so that it reserve
connection slots for addnoded nodes and tries to keep connecting to
them (or, alternatively, add a peernode flag for that behavior)
currently addnode is oneshot.

There is a lot of room for longer term improvements to the connection
and forwarding logic, and I have a couple interesting ones I'm running
on my nodes, but we don't really have any good way to test and
validate changes, so I'm hesitant to publish them.

Re: [Bitcoin-development] Useful bitcoin patches...

[Matt Corallo]

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

1) Native Long Polling Support
Not sure about this one, it would be useful, but preferably done once
Bitcoin is cleaned up anyway. A probable good way would be a central
notification interface which net, wallets, etc all connect to, including
things like a pool server. Thus I'm not sure about merging something
like this now.

2) A fix from Luke Dash Jr.
His branch is at
https://gitorious.org/~Luke-Jr/bitcoin/luke-jr-bitcoin/commits/getwork_dedupe and I would say it would be good to merge. gmaxwell indicated that hes been running this for a while and it is stable.

3) Hub Mode
Why? We dont have enough nodes listening to begin with, this generates a
ton of net usage on other listening nodes, which is not good. If you
open up your ports, you will get just as many, if not more connections
very easily.

4) RPC Turbocharge
Yep, this is something that is needed, there is also the other asio pull
request on github, not sure how/if these compare.

5) Resource Leak Fix
Looks good to me.

Matt

On Fri, 2011-07-01 at 18:03 +0200, Christian Decker wrote:
> Some appear to be beneficial to everybody.
> Multithreading the RPC will certainly speed up quite a few services
> and I see no downside in adding it. The same is true for Keep-Alive.
> 
> I'm against including the long polling support because incredibly few
> people will benefit from it (pool providers) and yet it is included
> for everyone.
> 
> The Hub mode is good, and I would go a step further and optimize the
> connection logic for all nodes by default. 
> 
> Just IMHO
> 
> Regards,
> Chris
> 
> 
> On Fri, Jul 1, 2011 at 5:23 AM, Jeff Garzik <jgarzik@exmulti.com>
> wrote:
>         This was posted to IRC:
>         http://davids.webmaster.com/~davids/bitcoin-3diff.txt
>         
>         Includes several useful features that all the big pools have
>         been
>         screaming for...  notably HTTP/1.1 keep-alive support.
>         
>         --
>         Jeff Garzik
>         exMULTI, Inc.
>         jgarzik@exmulti.com



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Bitcoin-development] Useful bitcoin patches...

[Luke-Jr]

On Thursday, June 30, 2011 11:23:56 PM Jeff Garzik wrote:
> This was posted to IRC:
> http://davids.webmaster.com/~davids/bitcoin-3diff.txt
> 
> Includes several useful features that all the big pools have been
> screaming for...  notably HTTP/1.1 keep-alive support.

"Native Long Polling Support" should be renamed. Real native long polling 
would be for solo miners.

"A fix from Luke Dash Jr." -- I have a branch with this; I didn't check that 
his version is up to date.

"RPC Turbocharge" -- If this incorporates only my changes for multithreading, 
it breaks JSON-RPC over SSL.

Re: [Bitcoin-development] Useful bitcoin patches...

[Luke-Jr]

On Thursday, June 30, 2011 11:23:56 PM Jeff Garzik wrote:
> This was posted to IRC:
> http://davids.webmaster.com/~davids/bitcoin-3diff.txt
> 
> Includes several useful features that all the big pools have been
> screaming for...  notably HTTP/1.1 keep-alive support.

There seems to be a new version at:
	http://davids.webmaster.com/~davids/bitcoin-4diff.txt
I haven't compared them yet.

For the "3diff" version, I extracted and made proper git branches for each 
logical part:
  hub_mode
  threaded_rpc
  \-- rpc_keepalive (depends on threaded_rpc, or a single connection would
                     keep the JSON-RPC interface locked up)
  signal_blk_notify (generic version of -pollpidfile, with a bugfix)
  bugfix_CreateThread_leak
  getwork_dedupe (original branch for my bugfix)

In addition, I also consider these branches valid candidates for merging:
  coinbaser (popens a given command and reads coinbase outputs from stdout)
  gitignore (ignore some common misc files)
  minfee_modes (internal function changes to allow specifying different fees
                for relay, send, or accept-in-block)
  \-- eligius_relay (relay lower fees only Eligius will accept)
      \-- eligius_sendfee (send lower fees only Eligius will accept)
  txinfo (adds entries to getinfo for transactions accepted for relaying,
          transactions accepted for the current block-in-progress, and current
          block-in-progress size)
  bitcoinuri (compliant support for all bitcoin: URIs)

All available from git://gitorious.org/~Luke-Jr/bitcoin/luke-jr-bitcoin.git

Re: [Bitcoin-development] Useful bitcoin patches...

[Matt Corallo]

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

On Sun, 2011-07-10 at 14:42 -0400, Luke-Jr wrote:
> On Thursday, June 30, 2011 11:23:56 PM Jeff Garzik wrote:
> > This was posted to IRC:
> > http://davids.webmaster.com/~davids/bitcoin-3diff.txt
> > 
> > Includes several useful features that all the big pools have been
> > screaming for...  notably HTTP/1.1 keep-alive support.
> 
> There seems to be a new version at:
> 	http://davids.webmaster.com/~davids/bitcoin-4diff.txt
> I haven't compared them yet.
> 
> For the "3diff" version, I extracted and made proper git branches for each 
> logical part:
>   hub_mode
No, no, no, no, no.  This has been discussed several times and provides
little to no advantage for miners and has the potential to severely harm
the network.
>   threaded_rpc
>   \-- rpc_keepalive (depends on threaded_rpc, or a single connection would
>                      keep the JSON-RPC interface locked up)
Some form of patch that implements these does need to be pulled soon, I
would say 0.4.1 or maybe sooner.
>   signal_blk_notify (generic version of -pollpidfile, with a bugfix)
Seems to be a feature for such a minority that until the codebase is
cleaned a ton we shouldn't add features for small minorities.  We have
seen even one or two line patches cause regressions so I, personally,
think we should really focus on cleaning the codebase (CWallet was a
great start) and then add all these minority features once the backend
stuff is really clean and efficient.
>   bugfix_CreateThread_leak
Yes, should be in for 0.4 and I think there is a pull request for it.
>   getwork_dedupe (original branch for my bugfix)
I think there is already a pull request, which should be merged for 0.4
IMO.
> 
> In addition, I also consider these branches valid candidates for merging:
>   coinbaser (popens a given command and reads coinbase outputs from stdout)
Seems like you are the only one who would benifit here, as noone else
but eligius changes coinbase output to a random set.
>   gitignore (ignore some common misc files)
>   minfee_modes (internal function changes to allow specifying different fees
>                 for relay, send, or accept-in-block)
We don't need something that just generically changes the functions to
allow whatever fee you want, we need something more generalized to allow
more custom settings, not just blind accept if fee is x per kb or
something.  Sipa has suggested various things that should allow for more
fee control by the users while still preventing users from sending
transactions that lock their coins in limbo.
>   \-- eligius_relay (relay lower fees only Eligius will accept)
>       \-- eligius_sendfee (send lower fees only Eligius will accept)
No, and no.  Just because you are willing to accept lower fees doesn't
mean the incentives are right to prevent DDoS.  The fees aren't there to
support the miners (not for a while, at least) they are there to prevent
stupid users from DDoSing and just generally wasting everyone else's
resources for no reason.
>   txinfo (adds entries to getinfo for transactions accepted for relaying,
>           transactions accepted for the current block-in-progress, and current
>           block-in-progress size)
These are cool numbers to know, but I'm not sure if they have any real
uses making them just useless feature creep.
>   bitcoinuri (compliant support for all bitcoin: URIs)
URI support would be nice.
> 
> All available from git://gitorious.org/~Luke-Jr/bitcoin/luke-jr-bitcoin.git
> 
> ------------------------------------------------------------------------------
> All of the data generated in your IT infrastructure is seriously valuable.
> Why? It contains a definitive record of application performance, security 
> threats, fraudulent activity, and more. Splunk takes this data and makes 
> sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-d2d-c2
> _______________________________________________
> Bitcoin-development mailing list
> Bitcoin-development@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bitcoin-development


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Bitcoin-development] Useful bitcoin patches...

[Luke-Jr]

On Sunday, July 10, 2011 3:12:12 PM Matt Corallo wrote:
> On Sun, 2011-07-10 at 14:42 -0400, Luke-Jr wrote:
> > For the "3diff" version, I extracted and made proper git branches for
> > each logical part:
> >   hub_mode
> 
> No, no, no, no, no.  This has been discussed several times and provides
> little to no advantage for miners and has the potential to severely harm
> the network.

I just said it exists. I don't expect anyone to promote or merge it.

> > In addition, I also consider these branches valid candidates for merging:
> >   coinbaser (popens a given command and reads coinbase outputs from
> >   stdout)
> 
> Seems like you are the only one who would benifit here, as noone else
> but eligius changes coinbase output to a random set.

I suspect because they haven't figured out how. Take it or leave it.

> >   minfee_modes (internal function changes to allow specifying different
> >                 fees for relay, send, or accept-in-block)
> 
> We don't need something that just generically changes the functions to
> allow whatever fee you want, we need something more generalized to allow
> more custom settings, not just blind accept if fee is x per kb or
> something.  Sipa has suggested various things that should allow for more
> fee control by the users while still preventing users from sending
> transactions that lock their coins in limbo.

This is a step in that direction, at least, by providing the mode as input.
Since 0.4 is moving to Qt, perhaps moving GetMinFee to QtScript is 
appropriate.

> >   \-- eligius_relay (relay lower fees only Eligius will accept)
> >       \-- eligius_sendfee (send lower fees only Eligius will accept)
> 
> No, and no.  Just because you are willing to accept lower fees doesn't
> mean the incentives are right to prevent DDoS.  The fees aren't there to
> support the miners (not for a while, at least) they are there to prevent
> stupid users from DDoSing and just generally wasting everyone else's
> resources for no reason.

Again, take it or leave it, but in the meantime you're asking for trouble from 
users who feel they're being forced to pay more than they have to. Or perhaps 
rather than trouble, that decision will increase awareness of other clients 
that don't try to control the users. That could be good too.

Re: [Bitcoin-development] Useful bitcoin patches...

[Matt Corallo]

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

On Sun, 2011-07-10 at 18:46 -0400, Luke-Jr wrote:
> On Sunday, July 10, 2011 6:12:43 PM Matt Corallo wrote:
> > On Sun, 2011-07-10 at 16:30 -0400, Luke-Jr wrote:
> > > Again, take it or leave it, but in the meantime you're asking for trouble
> > > from users who feel they're being forced to pay more than they have to.
> > > Or perhaps rather than trouble, that decision will increase awareness of
> > > other clients that don't try to control the users. That could be good
> > > too.
> > 
> > Its not a question of forcing users to pay anything, its a question of
> > how best to solve the "Im just gonna throw random crap in the chain for
> > the lulz" problem without causing too many side effects.  As with many
> > things Bitcoin, the temporary solution was something that was fairly
> > quickly hacked together to solve the problem without causing too many
> > problems.  What needs to happen is an actual solution, Bitcoin, in
> > theory, allows for all kinds of cool things, but the solution here needs
> > carefully thought out and implemented, not just keep lowering the fees.
> 
> The point is that we are already accepting lower fees. People (probably) want 
> to pay those lower fees. Yet there's developers writing their software to get 
> in the way between the user and pool willing to do business.
Read what I wrote again, and don't reply off-list.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Bitcoin-development] Useful bitcoin patches...

[Luke-Jr]

Reminder these are still ready for merging...

Jeff, you seem to have rewritten *part of* bugfix_CreateThread_leak and 
neglected to attribute the original author (as my branch did/does).

> For the "3diff" version, I extracted and made proper git branches for each
> logical part:
>   hub_mode
>   threaded_rpc
>   \-- rpc_keepalive (depends on threaded_rpc, or a single connection would
>                      keep the JSON-RPC interface locked up)
>   signal_blk_notify (generic version of -pollpidfile, with a bugfix)
>   bugfix_CreateThread_leak
>   getwork_dedupe (original branch for my bugfix)
> 
> In addition, I also consider these branches valid candidates for merging:
>   coinbaser (popens a given command and reads coinbase outputs from stdout)
>   gitignore (ignore some common misc files)
>   minfee_modes (internal function changes to allow specifying different
> fees for relay, send, or accept-in-block)
>   \-- eligius_relay (relay lower fees only Eligius will accept)
>       \-- eligius_sendfee (send lower fees only Eligius will accept)
>   txinfo (adds entries to getinfo for transactions accepted for relaying,
>           transactions accepted for the current block-in-progress, and
> current block-in-progress size)
>   bitcoinuri (compliant support for all bitcoin: URIs)
> 
> All available from git://gitorious.org/~Luke-Jr/bitcoin/luke-jr-bitcoin.git

Re: [Bitcoin-development] Useful bitcoin patches...

[Luke-Jr]

On Thursday, August 04, 2011 4:29:01 PM Luke-Jr wrote:
> Jeff, you seem to have rewritten *part of* bugfix_CreateThread_leak and
> neglected to attribute the original author (as my branch did/does).

My apologies. BlueMatt showed me how my comment here was wrong.
Hopefully I'll remember how GitHub shows merges like this in the future.

Re: [Bitcoin-development] Useful bitcoin patches...

[Jeff Garzik]

>>   threaded_rpc
>>   \-- rpc_keepalive (depends on threaded_rpc, or a single connection would
>>                      keep the JSON-RPC interface locked up)

Definitely want _some_ path to HTTP/1.1 persistent connections, either
this or async I/O, https://github.com/bitcoin/bitcoin/pull/214

Based on field usage, it sounds like the above, threaded rpc, is more
stable.  So my only question remaining, for which I've not yet
reviewed the code, is:  what is the behaviour under assault from lots
of new incoming TCP connections?  One very common problem with
threaded TCP servers is blindly creating new threads without any
filtering or checking of incoming connections, thereby quickly
reaching thread limits, possibly causing other parts of the program to
fail.

Any amount of RPC request volume, miner or not, sent to the present
implementation winds up using a lot of socket resources due to the
constant disconnect/reconnect demanded of the current HTTP/1.0 code.
This patch addresses that key issue, providing positive impact for all
production bitcoin sites using RPC.

Miners are hitting these problems now, and Gavin's right that mainnet
miners are a small portion of the total population, but our RPC server
implementation really is bloody awful at present.  Reliable,
production TCP servers use either threads or async I/O, or a
combination of both (thread pool + async I/O == win).  Deploying
HTTP/1.1 persistent connections will make a positive impact at any
site heavily using RPC.


>>   bugfix_CreateThread_leak

Did I leave something out, when merging

     commit 67ed7d9d4929d8fe1c5f976c184c72dff02d83b7
     Author: JoelKatz <DavidJoelSchwartz@GMail.com>
     Date:   Mon Jul 25 15:06:45 2011 -0700

?


>>   getwork_dedupe (original branch for my bugfix)

I think we need this, but have not yet reviewed/thought about it.

>>   gitignore (ignore some common misc files)

Sounds nice to have.

     Jeff


P.S.  I'm moving my home computer lab setup to a new house.  Internet
will be spotty until August 11, and you won't see much of me at all on
#bitcoin-dev.

Re: [Bitcoin-development] Useful bitcoin patches...

[Luke-Jr]

On Thursday, August 04, 2011 7:43:56 PM Jeff Garzik wrote:
> >>   bugfix_CreateThread_leak
> 
> Did I leave something out, when merging
> 
>      commit 67ed7d9d4929d8fe1c5f976c184c72dff02d83b7
>      Author: JoelKatz <DavidJoelSchwartz@GMail.com>
>      Date:   Mon Jul 25 15:06:45 2011 -0700
> 
> ?

Yes. I'm not sure it has any practical impact on the software, but there was 
what looks like a platform-specific compile fix (casting between potentially 
incompatible types) too.

[Bitcoin-development] Last try: Fixes for 0.4

[Luke-Jr]

Once more before I just give up on trying...

These branches fix bugs still remaining in 0.4rc1:
- bugfix_getwork_newblk_race
- getwork_dedupe
- boost_fs3
- gitignore

Additionally, this branch adds a tolerance to human error typing in base58 
with ambiguous symbols, and should be very safe to merge:
- base58_liberal_parsing

Again, these can all be merged with:
# git fetch git://gitorious.org/~Luke-Jr/bitcoin/luke-jr-bitcoin.git \
#           <branch name> && git merge FETCH_HEAD