New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove p2p alert system #7692

Merged
merged 7 commits into from Mar 21, 2016

Conversation

Projects
None yet
@btcdrak
Member

btcdrak commented Mar 15, 2016

This completely removes the p2p network alert messaging system; however, internal alerts, partition detection warnings and the -alertnotify option features remain.

The purpose of the p2p alert messaging system is to communicate severe network issues which can be achieved using a variety of traditional means rather than the Bitcoin p2p messaging layer. A decentralised system should not have privileged users able to send alert messages on the Bitcoin network.

From the perspective of the Bitcoin Core project, if we need to communicate with Core specific users, it can be done using existing public channels (website, twitter, reddit, Slack) as well as an opt-in Bitcoin Core announce only mailing list.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Mar 15, 2016

Member

Concept ACK.
Needs rebase.

Member

jonasschnelli commented Mar 15, 2016

Concept ACK.
Needs rebase.

@jonasschnelli jonasschnelli added the P2P label Mar 15, 2016

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Mar 15, 2016

Member

Concept ACK

Member

MarcoFalke commented Mar 15, 2016

Concept ACK

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak
Member

btcdrak commented Mar 15, 2016

@jonasschnelli rebased

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 15, 2016

Member

Concept ACK. I had an earlier try at this with #6260, but tt's good that there is an alternative in the form of a mailing list now - that was pretty much the only concern.

Member

laanwj commented Mar 15, 2016

Concept ACK. I had an earlier try at this with #6260, but tt's good that there is an alternative in the form of a mailing list now - that was pretty much the only concern.

@paveljanik

View changes

Show outdated Hide outdated src/test/alert_tests.cpp
@paveljanik

View changes

Show outdated Hide outdated src/test/alert_tests.cpp
@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Mar 15, 2016

Contributor

Concept ACK.

Contributor

paveljanik commented Mar 15, 2016

Concept ACK.

@maaku

This comment has been minimized.

Show comment
Hide comment
@maaku

maaku Mar 16, 2016

Contributor

This code is actually very useful for other projects that build off of bitcoin code base, and could be useful within the context of bitcoin if reconfigured. Perhaps just disable the code, not remove it entirely?

Contributor

maaku commented Mar 16, 2016

This code is actually very useful for other projects that build off of bitcoin code base, and could be useful within the context of bitcoin if reconfigured. Perhaps just disable the code, not remove it entirely?

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 16, 2016

Member

This code is actually very useful for other projects that build off of bitcoin code base

Sorry, but I don't think that's a valid reason to maintain code that we shouldn't have anymore. And I'm sure there's much better ways of doing this in derived projects as well, which don't rely on one network-wide secret key.

Member

laanwj commented Mar 16, 2016

This code is actually very useful for other projects that build off of bitcoin code base

Sorry, but I don't think that's a valid reason to maintain code that we shouldn't have anymore. And I'm sure there's much better ways of doing this in derived projects as well, which don't rely on one network-wide secret key.

@rebroad

This comment has been minimized.

Show comment
Hide comment
@rebroad

rebroad Mar 17, 2016

Contributor

concept ACK

Contributor

rebroad commented Mar 17, 2016

concept ACK

@achow101

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Mar 17, 2016

Member

So what has changed between now and several months ago when this was last attempted? Aren't the same reasons for not removing the alerts then still applicable today?

Member

achow101 commented Mar 17, 2016

So what has changed between now and several months ago when this was last attempted? Aren't the same reasons for not removing the alerts then still applicable today?

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 17, 2016

Member

Aren't the same reasons for not removing the alerts then still applicable today?

Just read #6260 and the OP.
A few months ago pretty much everyone was in favor of this, but there was no alternative notification system yet. There is now a mailing list for alerts instead.

The alert system suffers from many problems:

  • Philosophically, there should not be a key with special meaning on the P2P network, this has always been a sore point with other node implementations. Like the checkpoints, it is seen as a centralized point of control, thus should go. If there is to be a network-wide alert system, that would also need a network-wide bureaucracy for managing it.
  • The alert system is hardly tested and maintained. It opens an attack surface to people possessing a certain private key, even though it is a fairly small one, there may be some bug in the alert system that would turn it into a full blown backdoor.
  • It is not clear what kind of emergencies qualify for using it (there was no agreement on using it to warn of the UPnP issue, even though it was a local network code execution exploit).
  • It's possible that this guy has or had access to it:
 gpg: encrypted with 4096-bit RSA key, ID EACB3C76, created 2010-07-22
       "Mark Karpelès <mark@hell.ne.jp>"

(and Satoshi, and possibly others who shouldn't really be able to)

A notification mailing list doesn't have any of these problems - it will be about announcements and alerts about this specific software, and we can directly control who has post access.

Note that I'm in no way against a 'better' alert system later on, such as one that doesn't rely on a special P2P message. There are some suggestions in #6260. But this one should go, and soon.

Member

laanwj commented Mar 17, 2016

Aren't the same reasons for not removing the alerts then still applicable today?

Just read #6260 and the OP.
A few months ago pretty much everyone was in favor of this, but there was no alternative notification system yet. There is now a mailing list for alerts instead.

The alert system suffers from many problems:

  • Philosophically, there should not be a key with special meaning on the P2P network, this has always been a sore point with other node implementations. Like the checkpoints, it is seen as a centralized point of control, thus should go. If there is to be a network-wide alert system, that would also need a network-wide bureaucracy for managing it.
  • The alert system is hardly tested and maintained. It opens an attack surface to people possessing a certain private key, even though it is a fairly small one, there may be some bug in the alert system that would turn it into a full blown backdoor.
  • It is not clear what kind of emergencies qualify for using it (there was no agreement on using it to warn of the UPnP issue, even though it was a local network code execution exploit).
  • It's possible that this guy has or had access to it:
 gpg: encrypted with 4096-bit RSA key, ID EACB3C76, created 2010-07-22
       "Mark Karpelès <mark@hell.ne.jp>"

(and Satoshi, and possibly others who shouldn't really be able to)

A notification mailing list doesn't have any of these problems - it will be about announcements and alerts about this specific software, and we can directly control who has post access.

Note that I'm in no way against a 'better' alert system later on, such as one that doesn't rely on a special P2P message. There are some suggestions in #6260. But this one should go, and soon.

@achow101

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Mar 17, 2016

Member

A mailing list would work for this specific client, but what about network wide issues like a blockchain fork like the fourth of July fork?

Also, since the alert system is network wide, what will be done about other clients that still implement the alerts?

Member

achow101 commented Mar 17, 2016

A mailing list would work for this specific client, but what about network wide issues like a blockchain fork like the fourth of July fork?

Also, since the alert system is network wide, what will be done about other clients that still implement the alerts?

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 17, 2016

Member

A mailing list would work for this specific client, but what about network wide issues like a blockchain fork like the fourth of July fork?

Network wide issues will also be posted to the mailing list. Also, other software can have their own mailing lists. Decentralization, you know. No one should be trusted with central responsibility to send alerts over the network.

what will be done about other clients that still implement the alerts?

They'll likely remove the code as well. Or not. In any case it will never be triggered again. It was never very useful for other clients, as they couldn't send messages of themselves (see #5160).

Member

laanwj commented Mar 17, 2016

A mailing list would work for this specific client, but what about network wide issues like a blockchain fork like the fourth of July fork?

Network wide issues will also be posted to the mailing list. Also, other software can have their own mailing lists. Decentralization, you know. No one should be trusted with central responsibility to send alerts over the network.

what will be done about other clients that still implement the alerts?

They'll likely remove the code as well. Or not. In any case it will never be triggered again. It was never very useful for other clients, as they couldn't send messages of themselves (see #5160).

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Mar 17, 2016

Member

@achow101 Please note the alert system was not even used for the "July fork".

If Mark Karpeles has the key, how do we know he wasn't forced to hand it over to the Japanese police or that they have obtained it from accessing his computers? At this stage the key should be considered compromised at the very least, but in any case, a network wide, privileged messaging system is pretty outrageous for Bitcoin. It might have been a reasonable compromise in the early days, but we've definitely outgrown the need now.

Member

btcdrak commented Mar 17, 2016

@achow101 Please note the alert system was not even used for the "July fork".

If Mark Karpeles has the key, how do we know he wasn't forced to hand it over to the Japanese police or that they have obtained it from accessing his computers? At this stage the key should be considered compromised at the very least, but in any case, a network wide, privileged messaging system is pretty outrageous for Bitcoin. It might have been a reasonable compromise in the early days, but we've definitely outgrown the need now.

@jl2012

This comment has been minimized.

Show comment
Hide comment
@jl2012

jl2012 Mar 17, 2016

Member

Concept ACK

Member

jl2012 commented Mar 17, 2016

Concept ACK

@NicolasDorier

This comment has been minimized.

Show comment
Hide comment
@NicolasDorier

NicolasDorier Mar 17, 2016

Member

Concept ACK

Member

NicolasDorier commented Mar 17, 2016

Concept ACK

@achow101

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Mar 17, 2016

Member

@btcdrak Wait, it wasn't used in that fork? I thought it was.

Anyways, since it looks like there are better alternatives which allow for more decentralization, I agree with removing this.
Concept ACK.

Although, if/when this is merged, all of the other wallet developers should be informed so that they remove the code for processing alerts.

Also, maybe the community should be made aware of this decision since this is a protocol rule. I think that if this was merged without letting other people "vote" or debate this, it would probably result in a shitstorm about "The core developers are taking too much power by forcing protocol rules".

Member

achow101 commented Mar 17, 2016

@btcdrak Wait, it wasn't used in that fork? I thought it was.

Anyways, since it looks like there are better alternatives which allow for more decentralization, I agree with removing this.
Concept ACK.

Although, if/when this is merged, all of the other wallet developers should be informed so that they remove the code for processing alerts.

Also, maybe the community should be made aware of this decision since this is a protocol rule. I think that if this was merged without letting other people "vote" or debate this, it would probably result in a shitstorm about "The core developers are taking too much power by forcing protocol rules".

@instagibbs

This comment has been minimized.

Show comment
Hide comment
@instagibbs

instagibbs Mar 17, 2016

Member

Concept ACK

Member

instagibbs commented Mar 17, 2016

Concept ACK

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 17, 2016

Member

"The core developers are taking too much power by forcing protocol rules".

We're removing our own privileged position from the P2P protocol (note: not consensus) rules. Oh no! Taking so much power.

Member

laanwj commented Mar 17, 2016

"The core developers are taking too much power by forcing protocol rules".

We're removing our own privileged position from the P2P protocol (note: not consensus) rules. Oh no! Taking so much power.

@achow101

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Mar 17, 2016

Member

We're removing our own privileged position from the P2P protocol (note: not consensus) rules. Oh no!

Yeah, and people can be irrational and there are also shills and conspiracy theorists trying to find every reason to discredit the Core devs

Member

achow101 commented Mar 17, 2016

We're removing our own privileged position from the P2P protocol (note: not consensus) rules. Oh no!

Yeah, and people can be irrational and there are also shills and conspiracy theorists trying to find every reason to discredit the Core devs

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Mar 17, 2016

Member

@achow101 This is not a consensus rule. We are choosing to remove centralisation from the Bitcoin Core distribution.

Member

btcdrak commented Mar 17, 2016

@achow101 This is not a consensus rule. We are choosing to remove centralisation from the Bitcoin Core distribution.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 17, 2016

Member

Yeah, and people can be irrational and there are also shills and conspiracy theorists trying to find every reason to discredit the Core devs

This is going very far off-topic. Let's keep it at this.

Member

laanwj commented Mar 17, 2016

Yeah, and people can be irrational and there are also shills and conspiracy theorists trying to find every reason to discredit the Core devs

This is going very far off-topic. Let's keep it at this.

@achow101

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Mar 17, 2016

Member

@btcdrak Yes, I know. I am just saying that the reaction to this will probably be that even though it is a protocol rule.

@laanwj sorry (I've been hanging out at bitcointalk too long)

Member

achow101 commented Mar 17, 2016

@btcdrak Yes, I know. I am just saying that the reaction to this will probably be that even though it is a protocol rule.

@laanwj sorry (I've been hanging out at bitcointalk too long)

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Mar 17, 2016

Member

I'd prefer to see an equivalent alert system replacement first, but the risks to the current one are probably significant enough to warrant its early removal.

Member

luke-jr commented Mar 17, 2016

I'd prefer to see an equivalent alert system replacement first, but the risks to the current one are probably significant enough to warrant its early removal.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 18, 2016

Member

I'd prefer to see an equivalent alert system replacement first

That was the same argument last time, and the time before that. At some point we have to cut the knot, and I'd say that is now.

Lots of proposals for alternatives, but an alert system isn't really anything people want to spend time working on, it appears. It makes sense in a way, because the only time people worry about such a system is right when they need it.

Hopefully removal will prompt people to work on something better. Although I sincerely believe a mailing list will do a better job than what we have now.

Member

laanwj commented Mar 18, 2016

I'd prefer to see an equivalent alert system replacement first

That was the same argument last time, and the time before that. At some point we have to cut the knot, and I'd say that is now.

Lots of proposals for alternatives, but an alert system isn't really anything people want to spend time working on, it appears. It makes sense in a way, because the only time people worry about such a system is right when they need it.

Hopefully removal will prompt people to work on something better. Although I sincerely believe a mailing list will do a better job than what we have now.

@seweso

This comment has been minimized.

Show comment
Hide comment
@seweso

seweso Mar 18, 2016

This would mean you move to even more centralised communication methods. So it is very weird to use decentralisation as an argument here.

Not to mention that the alert system gives information at exactly the right time: when you plan to use Bitcoin.

My advice would be to de-activate it at a certain block height, and then remove it. That should add enough pressure to build alternative, and give enough time to do so.

I also missed the discussion about this, was there any?

seweso commented Mar 18, 2016

This would mean you move to even more centralised communication methods. So it is very weird to use decentralisation as an argument here.

Not to mention that the alert system gives information at exactly the right time: when you plan to use Bitcoin.

My advice would be to de-activate it at a certain block height, and then remove it. That should add enough pressure to build alternative, and give enough time to do so.

I also missed the discussion about this, was there any?

@Aquentus

This comment has been minimized.

Show comment
Hide comment
@Aquentus

Aquentus Mar 18, 2016

Wasn't an Alert sent to all nodes in 2013 to ask them to downgrade to 0.7 urgently? I think there are some irc chat logs which show that there was. Can we say, in the absence of the alert, how much longer it may take for such accidental hardfork to be quickly resolved?

Although other public announcement methods can be used, node operators may not be paying attention at that specific point, with the alert probably being the most direct way of reaching them.

I'm not necessarily against removing the alert, but I think there should first be some analysis of the effect its removal may have in times of emergencies. Would it, for example, mean that an accidental hardfork may go on for days rather than hours?

In regards to the suggestion that it is a centralised point, I do agree to an extent and individuals like MK for example should definitely not have the alert, but at the point of misuse the alert system can be revoked, thus achieving what is proposed. Until then, I am not sure what harm the alert system can do? A potential backdoor? Perhaps, but I don't see how and it sounds like high speculation with no basis. There "could" be a "potential" backdoor in every part of the code.

Even assuming the Japanese police has this key, what damage can they do when any alert they may send would be instantly revoked?

So it's a NACK from me until full analysis of the effect of removing the alert would have on emergency situations. For example, if it means that an accidental hardfork would last for days, I think that would be a disaster so I wouldn't support it's removal.

Aquentus commented Mar 18, 2016

Wasn't an Alert sent to all nodes in 2013 to ask them to downgrade to 0.7 urgently? I think there are some irc chat logs which show that there was. Can we say, in the absence of the alert, how much longer it may take for such accidental hardfork to be quickly resolved?

Although other public announcement methods can be used, node operators may not be paying attention at that specific point, with the alert probably being the most direct way of reaching them.

I'm not necessarily against removing the alert, but I think there should first be some analysis of the effect its removal may have in times of emergencies. Would it, for example, mean that an accidental hardfork may go on for days rather than hours?

In regards to the suggestion that it is a centralised point, I do agree to an extent and individuals like MK for example should definitely not have the alert, but at the point of misuse the alert system can be revoked, thus achieving what is proposed. Until then, I am not sure what harm the alert system can do? A potential backdoor? Perhaps, but I don't see how and it sounds like high speculation with no basis. There "could" be a "potential" backdoor in every part of the code.

Even assuming the Japanese police has this key, what damage can they do when any alert they may send would be instantly revoked?

So it's a NACK from me until full analysis of the effect of removing the alert would have on emergency situations. For example, if it means that an accidental hardfork would last for days, I think that would be a disaster so I wouldn't support it's removal.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 18, 2016

Member

This would mean you move to even more centralised communication methods. So it is very weird to use decentralisation as an argument here.

Decentralization is always a compromise, you get more of something, but all of those instances are of course centralized, controlled by one or a few persons (for example, nodes). It's more decentralized in this way: every project (Core, Btcd, Classic, etc) can have its own notification system, there is no more 'global' system where a few people have a golden key.

This is exactly how it should be - how can you call Core, having its own notification system controlled by Core developers, overly centralized? This is the project. And if you're not using Core, then you shouldn't even be arguing here!

I also missed the discussion about this, was there any?

#6260 at least.

Member

laanwj commented Mar 18, 2016

This would mean you move to even more centralised communication methods. So it is very weird to use decentralisation as an argument here.

Decentralization is always a compromise, you get more of something, but all of those instances are of course centralized, controlled by one or a few persons (for example, nodes). It's more decentralized in this way: every project (Core, Btcd, Classic, etc) can have its own notification system, there is no more 'global' system where a few people have a golden key.

This is exactly how it should be - how can you call Core, having its own notification system controlled by Core developers, overly centralized? This is the project. And if you're not using Core, then you shouldn't even be arguing here!

I also missed the discussion about this, was there any?

#6260 at least.

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak
Member

btcdrak commented Mar 18, 2016

@paveljanik done.

@laanwj laanwj added this to the 0.13.0 milestone Mar 18, 2016

@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Mar 19, 2016

Member

utACK. This should be removed.

3 people have the keys

Many more people than three have the keys; the complete set is not made public for personal safety reasons (and given the likely compromise, is not even known to any person). But now, understanding that this misunderstanding exists, some of the strange opposition makes a lot more sense to me, and this only increases my belief that this should be removed.

Member

gmaxwell commented Mar 19, 2016

utACK. This should be removed.

3 people have the keys

Many more people than three have the keys; the complete set is not made public for personal safety reasons (and given the likely compromise, is not even known to any person). But now, understanding that this misunderstanding exists, some of the strange opposition makes a lot more sense to me, and this only increases my belief that this should be removed.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Mar 21, 2016

Contributor

concept ACK

Contributor

dcousens commented Mar 21, 2016

concept ACK

@laanwj laanwj merged commit cfd519e into bitcoin:master Mar 21, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Mar 21, 2016

Merge #7692: Remove p2p alert system
cfd519e Add release note documentation (BtcDrak)
6601ce5 protocol.h/cpp: Removes NetMsgType::ALERT (Thomas Kerin)
ad72104 Formatting (BtcDrak)
1b77471 Remove alert keys (BtcDrak)
01fdfef Remove `-alerts` option (BtcDrak)
9206634 Update alert notification and GUI (BtcDrak)
bbb9d1d Remove p2p alert handling (BtcDrak)
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj
Member

laanwj commented Mar 21, 2016

ACK cfd519e

@achow101 achow101 referenced this pull request Mar 22, 2016

Closed

Remove p2p alert system #27

@dgenr8 dgenr8 referenced this pull request Mar 22, 2016

Closed

Remove p2p alert system #138

@whatisgravity

This comment has been minimized.

Show comment
Hide comment
@whatisgravity

whatisgravity Mar 24, 2016

The fact that this introduces a greater attack surface for an unknown group of people should be enough to remove it immediately.

Edit: Also isn't there clear conflict of interest issues with anyone who holds a key from arguing/voting on the existence of this feature?

This code is actually very useful for other projects that build off of bitcoin code base

They can look at previous commits, thats the point of version control.

whatisgravity commented Mar 24, 2016

The fact that this introduces a greater attack surface for an unknown group of people should be enough to remove it immediately.

Edit: Also isn't there clear conflict of interest issues with anyone who holds a key from arguing/voting on the existence of this feature?

This code is actually very useful for other projects that build off of bitcoin code base

They can look at previous commits, thats the point of version control.

@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Mar 24, 2016

Member

@whatisgravity It's removed now-- it's worth noting that the main contributors to Bitcoin Core have been trying to remove it for a couple years now, but have (and continue to) suffered pushback from some parties... it took a while to overcome that.

Member

gmaxwell commented Mar 24, 2016

@whatisgravity It's removed now-- it's worth noting that the main contributors to Bitcoin Core have been trying to remove it for a couple years now, but have (and continue to) suffered pushback from some parties... it took a while to overcome that.

@maaku

This comment has been minimized.

Show comment
Hide comment
@maaku

maaku Mar 24, 2016

Contributor

Sorry, but I don't think that's a valid reason to maintain code that we shouldn't have anymore. And I'm sure there's much better ways of doing this in derived projects as well, which don't rely on one network-wide secret key.

That's the wonder of open source -- having code in a repository doesn't mean that you or the other core committers are required to personally support it, other than make sure that your own merged patches don't break automated unit tests.

If the features of the bitcoin core repository are limited to those which some subset of developers are specifically interested in supporting, it makes bitcoin core a rather uninteresting project to the wider community.

Contributor

maaku commented Mar 24, 2016

Sorry, but I don't think that's a valid reason to maintain code that we shouldn't have anymore. And I'm sure there's much better ways of doing this in derived projects as well, which don't rely on one network-wide secret key.

That's the wonder of open source -- having code in a repository doesn't mean that you or the other core committers are required to personally support it, other than make sure that your own merged patches don't break automated unit tests.

If the features of the bitcoin core repository are limited to those which some subset of developers are specifically interested in supporting, it makes bitcoin core a rather uninteresting project to the wider community.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 25, 2016

Member

That's the wonder of open source -- having code in a repository doesn't mean that you or the other core committers are required to personally support it, other than make sure that your own merged patches don't break automated unit tests.

No, I disagree - at least how our project is structured - trying hard to handle issues and fix bugs that come up, for example - there is at least a little responsibility to the maintainers for what is in the repository.

Only passing the automated tests is short-sighted. At least as long as the automated tests don't cover everything on every scenario on every platform (and some things, like people that act in unpredictable ways, can hardly be covered by automated tests).

I do agree that you could structure an open source project that way. We're hampered also by the monolithic structure of the code. E.g. if the alert system was an external plugin, people who care about it could still maintain it, and we'd only have to make sure that our side of the API does what is advertised. But for better or worse, we have this bottleneck.

it makes bitcoin core a rather uninteresting project to the wider community.

Possibly. But on the other hand, what we do support we try to keep working as well as possible. It's a bit of a compromise, where on one side you have a heap of barely-third-party-maintained hacks and on the other side you have a cathedral. I try to keep to a sensible middle, as said above, as far as the code structure allows.

Member

laanwj commented Mar 25, 2016

That's the wonder of open source -- having code in a repository doesn't mean that you or the other core committers are required to personally support it, other than make sure that your own merged patches don't break automated unit tests.

No, I disagree - at least how our project is structured - trying hard to handle issues and fix bugs that come up, for example - there is at least a little responsibility to the maintainers for what is in the repository.

Only passing the automated tests is short-sighted. At least as long as the automated tests don't cover everything on every scenario on every platform (and some things, like people that act in unpredictable ways, can hardly be covered by automated tests).

I do agree that you could structure an open source project that way. We're hampered also by the monolithic structure of the code. E.g. if the alert system was an external plugin, people who care about it could still maintain it, and we'd only have to make sure that our side of the API does what is advertised. But for better or worse, we have this bottleneck.

it makes bitcoin core a rather uninteresting project to the wider community.

Possibly. But on the other hand, what we do support we try to keep working as well as possible. It's a bit of a compromise, where on one side you have a heap of barely-third-party-maintained hacks and on the other side you have a cathedral. I try to keep to a sensible middle, as said above, as far as the code structure allows.

chjj added a commit to bcoin-org/bcoin that referenced this pull request Aug 25, 2016

@btcdrak btcdrak deleted the btcdrak:remove_alert branch Dec 3, 2016

@kyuupichan kyuupichan referenced this pull request Mar 11, 2017

Merged

Removal of alert system #360

kyuupichan referenced this pull request in kyuupichan/BitcoinUnlimited Mar 20, 2017

Merge #7692: Remove p2p alert system
cfd519e Add release note documentation (BtcDrak)
6601ce5 protocol.h/cpp: Removes NetMsgType::ALERT (Thomas Kerin)
ad72104 Formatting (BtcDrak)
1b77471 Remove alert keys (BtcDrak)
01fdfef Remove `-alerts` option (BtcDrak)
9206634 Update alert notification and GUI (BtcDrak)
bbb9d1d Remove p2p alert handling (BtcDrak)

sickpig referenced this pull request in sickpig/BitcoinUnlimited Mar 31, 2017

Merge #7692: Remove p2p alert system
cfd519e Add release note documentation (BtcDrak)
6601ce5 protocol.h/cpp: Removes NetMsgType::ALERT (Thomas Kerin)
ad72104 Formatting (BtcDrak)
1b77471 Remove alert keys (BtcDrak)
01fdfef Remove `-alerts` option (BtcDrak)
9206634 Update alert notification and GUI (BtcDrak)
bbb9d1d Remove p2p alert handling (BtcDrak)

@bokobza bokobza referenced this pull request Jun 10, 2018

Merged

Removed alerts #1496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment