Discussion:
[bitcoin-dev] libconsensus and bitcoin development process
Jeff Garzik via bitcoin-dev
2015-09-15 04:10:37 UTC
Permalink
[collating a private mail and a github issue comment, moving it to a
better forum]

On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.

To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.

I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.

I read every code change in every pull request that comes into
github/bitcoin/bitcoin with three exceptions:
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes

As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.

Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.

It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.

Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
folks to get trapped between a rock and a hard place:
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.

Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.


Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.

The general goal is good. But we must not stray into unfocused
engineering for a non-existent future library user.

The higher priority must be given to having a source code base that
maximizes the collective developers' ability to maintain The Router --
the core bitcoin full node P2P engine.

I recommend time-based bursts of code movement changes. See below;
for example, just submit & merge code movement changes on the first
week of every 2nd month. Code movement changes are easy to create
from scratch once a concrete goal is known. The coding part is
trivial and takes no time.

As we saw in the Linux kernel - battle lessons hard learned - code
movement and refactors have often unseen negative impact on downstream
developers working on more complicated changes that have more positive
impact to our developers and users.


On Bitcoin development release cycles & process
------------------------------------------------------------------

As I've outlined in the past, the Linux kernel maintenance phases
address some of these problems. The merge window into git master
opens for 1 week, a very chaotic week full of merging (and rebasing),
and then the merge window closes. Several weeks follow as the "dust
settles" -- testing, bug fixing, moving in parallel OOB with
not-yet-ready development. Release candidates follow, then the
release, then the cycle repeats.

IMO a merge window approach fixes some of the issues with refactoring,
as well as introduces some useful -developer discipline- into the
development process. Bitcoin Core still needs rapid iteration --
another failing of the current project -- and so something of a more
rapid pace is needed:
- 1st week of each month, merge changes. Lots of rebasing during this week.
- remaining days of the month, test, bug fix
- release at end of month

If changes are not ready for merging, then so be it, they wait until
next month's release. Some releases have major features, some
releases are completely boring and offer little of note. That is the
nature of time-based development iteration. It's like dollar cost
averaging, a bit.


And frankly, I would like to close all github pull requests that are
not ready to merge That Week. I'm as guilty of this as any, but that
stuff just languishes. Excluding a certain category of obvious-crap,
pull requests tend to default to a state of either (a) rapid merging,
(b) months-long issues/projects, (c) limbo.

Under a more time-based approach, a better pull request process would be to
* Only open pull requests if it's a bug fix, or the merge window is
open and the change is ready to be merged in the developer's opinion.
* Developers CC bitcoin-dev list to discuss Bitcoin Core-bound projects
* Developers maintain and publish projects via their own git trees
* Pull requests should be closed if unmerged after 7 days, unless it
is an important bug fix etc.

The problem with projects like libconsensus is that they can get
unfocused and open ended. Code movement changes in particular are
cheap to generate. It is low developer cost for the developer to
iterate all the way to the end state, see what that looks like, and
see if people like it. That end state is not something you would
merge all in one go. I would likely stash that tree, and then start
again, seek the most optimal and least disruptive set of refactors,
and generate and merge those into bitcoin/bitcoin.git in a time-based,
paced manner. Announce the pace ahead of time - "cosmetic stuff that
breaks your patches will be merged 1st week of every second month"

To underscore, the higher priority must be given to having a source
code base and disciplined development process that maximizes the
collective developers' ability to maintain The Router that maintains
most of our network.

Modularity, refactoring, cleaning up grotty code generates a deep
seated happiness in many engineers. Field experience however shows
refactoring is a never ending process which sometimes gets in the way
of More Important Work.
Btc Drak via bitcoin-dev
2015-09-15 09:55:34 UTC
Permalink
I also share a lot of Jeff's concerns about refactoring and have voiced
them several times on IRC and in private to Jorge, Wladamir and Greg. I
meant to do a write up but never got around to it. Jeff has quite
eloquently stated the various problems. I would like to share my thoughts
on the matter because we really do need to come up with a plan on how this
issue is dealt with.

Obviously, Bitcoin Core is quite tightly coupled at the moment and
definitely needs extensive modularisation. Such work will inevitably
require lots of bulk code moves and then finer refactoring. However, it
requires proper planning because there are lots of effects and consequences
for other people contributing to Core and also downstream projects relying
on Core:

1. Refactoring often causes other pull requests to diverge and require
rebasing. Continual refactoring can put PRs in "rebase hell" and puts a big
stress on contributors (many of whom are part time).

2. Version to version, Bitcoin Core changes significantly in structure. 0.9
to 0.10 is unrecognisable. 0.10 to 0.11 is even more so. This makes makes
it hard to follow release to release and the net result is less people
upgrade (especially think of miners trying to keep their patch sets working
while trying not to disrupt or risk their mining operations).

3. Continual refactoring increases risk: we're human, and mistakes will
slip through peer review. This is especially concerning with consensus
critical code and this makes it difficult to merge such refactoring often,
which of course exacerbates the problem.

The net negative consequence is it is harder to contribute to Core, harder
for the Core maintainers to merge and harder for downstream/dependent
projects/implementations to keep up.

Suggested Way Forward
---------------------------------

With the understanding that refactored code by definition must not change
behaviour. There are three major kinds of refactoring:

1. code moves (e.g. separating concerns into different files);
2. code style;
3. structural optimisation and consolidation (reducing LOC, separating
concerns, encapsulation etc).

Code moves(1) and CS(2) are easy to peer review and merge quickly. The
third kind(3) requires deeper analysis to ensure that while the code
changed, the behaviour (including any bugs) did not.

We must resist all temptation to fix bugs or tack on minor fixes and tweaks
during refactoring: pull requests should only be refactoring only, with no
net change to behaviour. Keeping discipline makes it much easier to verify
and peer review and this faster to merge.

With respect to Code moves and CS, I believe we should have a "refactoring
fortnight" where we so the bulk of code move-only refactoring plus CS where
necessary. This is by fat the most disruptive kind of change because it
widely affects other PRs mergeability. We should aim to get most of this
done in one go, so that it's not happening in dribs and drabs over months
and many releases. Once done, it gives everyone a good idea to the overall
new structure and where one can expect to find things in the future. The
idea here is to help orientation and not have to continuously hunt for
where things have moved to.

To be clear, I am strongly suggesting code move-only refactoring PRs not be
mixed with anything else. Same for CS changes. This makes the PRs extremely
easy to vet and thus quick to merge.

Towards this end, maybe there should be an IRC meeting to agree the initial
moves, then someone who has the stomach for it can get on and do it -
during that time, we do not merge anything else. We need to bite the bullet
and break the back out of code moves.

With regards to CS, I think we do need to get CS right, because a continual
dribble of CS changes also makes diffs between releases less easy to
follow. Much of CS checking can be automated by the continuous integration
so authors can get it right easily. It can be just like a Travis check.

With respect to the 3rd kind of refactoring, we need to set some standards
and goals and aim for some kind of consistency. Refactoring needs to fulfil
certain goals and criterion otherwise contributors will always find a
reason to fiddle over and over forever. Obvious targets here can be things
like proper encapsulation and separation of concerns.

Overall, refactoring should be merged quickly, but only on a schedule so it
doesn't cause major disruption to others.

Obviously the third kind of refactoring more complex and time consuming and
will need to occur over time, but it should happen in defined steps. As
Jeff said, one week a month, or maybe one month a release. In any case,
refactoring changes should be quickly accepted or rejected by the project
maintainer and not left hanging.

Finally, refactoring should *always* be uncontroversial because essentially
functionality is not changing. If functionality changes (e.g. you try to
sneak in a big fix or feature tweak "because it's small") the PR should be
rejected outright. Additionally, if we break down refactoring into the
three kinds stated above, peer review will be much more straightforward.



On Tue, Sep 15, 2015 at 5:10 AM, Jeff Garzik via bitcoin-dev <
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
The general goal is good. But we must not stray into unfocused
engineering for a non-existent future library user.
The higher priority must be given to having a source code base that
maximizes the collective developers' ability to maintain The Router --
the core bitcoin full node P2P engine.
I recommend time-based bursts of code movement changes. See below;
for example, just submit & merge code movement changes on the first
week of every 2nd month. Code movement changes are easy to create
from scratch once a concrete goal is known. The coding part is
trivial and takes no time.
As we saw in the Linux kernel - battle lessons hard learned - code
movement and refactors have often unseen negative impact on downstream
developers working on more complicated changes that have more positive
impact to our developers and users.
On Bitcoin development release cycles & process
------------------------------------------------------------------
As I've outlined in the past, the Linux kernel maintenance phases
address some of these problems. The merge window into git master
opens for 1 week, a very chaotic week full of merging (and rebasing),
and then the merge window closes. Several weeks follow as the "dust
settles" -- testing, bug fixing, moving in parallel OOB with
not-yet-ready development. Release candidates follow, then the
release, then the cycle repeats.
IMO a merge window approach fixes some of the issues with refactoring,
as well as introduces some useful -developer discipline- into the
development process. Bitcoin Core still needs rapid iteration --
another failing of the current project -- and so something of a more
- 1st week of each month, merge changes. Lots of rebasing during this week.
- remaining days of the month, test, bug fix
- release at end of month
If changes are not ready for merging, then so be it, they wait until
next month's release. Some releases have major features, some
releases are completely boring and offer little of note. That is the
nature of time-based development iteration. It's like dollar cost
averaging, a bit.
And frankly, I would like to close all github pull requests that are
not ready to merge That Week. I'm as guilty of this as any, but that
stuff just languishes. Excluding a certain category of obvious-crap,
pull requests tend to default to a state of either (a) rapid merging,
(b) months-long issues/projects, (c) limbo.
Under a more time-based approach, a better pull request process would be to
* Only open pull requests if it's a bug fix, or the merge window is
open and the change is ready to be merged in the developer's opinion.
* Developers CC bitcoin-dev list to discuss Bitcoin Core-bound projects
* Developers maintain and publish projects via their own git trees
* Pull requests should be closed if unmerged after 7 days, unless it
is an important bug fix etc.
The problem with projects like libconsensus is that they can get
unfocused and open ended. Code movement changes in particular are
cheap to generate. It is low developer cost for the developer to
iterate all the way to the end state, see what that looks like, and
see if people like it. That end state is not something you would
merge all in one go. I would likely stash that tree, and then start
again, seek the most optimal and least disruptive set of refactors,
and generate and merge those into bitcoin/bitcoin.git in a time-based,
paced manner. Announce the pace ahead of time - "cosmetic stuff that
breaks your patches will be merged 1st week of every second month"
To underscore, the higher priority must be given to having a source
code base and disciplined development process that maximizes the
collective developers' ability to maintain The Router that maintains
most of our network.
Modularity, refactoring, cleaning up grotty code generates a deep
seated happiness in many engineers. Field experience however shows
refactoring is a never ending process which sometimes gets in the way
of More Important Work.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
Jeff Garzik via bitcoin-dev
2015-09-15 15:26:50 UTC
Permalink
Drak,

I would say that the refactoring does actually fulfill some conditions you
mention:
- move-only is almost always clearly separated out
- the refactoring is not controversial _in minimis_ - meaning, the
individual pull request is not controversial.

The problem comes with the impact of an unfocused stream of refactors to
key code.

For example, there is much less long term developer impact if refactoring
were _accelerated_, scheduled to be performed in a one-week sprint. There
is a lot of breakage, yes, but after that week the average level of
downstream patch breakage is significantly lower. A "rip the band-aid off
quickly rather than slowly" approach.
Post by Btc Drak via bitcoin-dev
I also share a lot of Jeff's concerns about refactoring and have voiced
them several times on IRC and in private to Jorge, Wladamir and Greg. I
meant to do a write up but never got around to it. Jeff has quite
eloquently stated the various problems. I would like to share my thoughts
on the matter because we really do need to come up with a plan on how this
issue is dealt with.
Obviously, Bitcoin Core is quite tightly coupled at the moment and
definitely needs extensive modularisation. Such work will inevitably
require lots of bulk code moves and then finer refactoring. However, it
requires proper planning because there are lots of effects and consequences
for other people contributing to Core and also downstream projects relying
1. Refactoring often causes other pull requests to diverge and require
rebasing. Continual refactoring can put PRs in "rebase hell" and puts a big
stress on contributors (many of whom are part time).
2. Version to version, Bitcoin Core changes significantly in structure.
0.9 to 0.10 is unrecognisable. 0.10 to 0.11 is even more so. This makes
makes it hard to follow release to release and the net result is less
people upgrade (especially think of miners trying to keep their patch sets
working while trying not to disrupt or risk their mining operations).
3. Continual refactoring increases risk: we're human, and mistakes will
slip through peer review. This is especially concerning with consensus
critical code and this makes it difficult to merge such refactoring often,
which of course exacerbates the problem.
The net negative consequence is it is harder to contribute to Core, harder
for the Core maintainers to merge and harder for downstream/dependent
projects/implementations to keep up.
Suggested Way Forward
---------------------------------
With the understanding that refactored code by definition must not change
1. code moves (e.g. separating concerns into different files);
2. code style;
3. structural optimisation and consolidation (reducing LOC, separating
concerns, encapsulation etc).
Code moves(1) and CS(2) are easy to peer review and merge quickly. The
third kind(3) requires deeper analysis to ensure that while the code
changed, the behaviour (including any bugs) did not.
We must resist all temptation to fix bugs or tack on minor fixes and
tweaks during refactoring: pull requests should only be refactoring only,
with no net change to behaviour. Keeping discipline makes it much easier to
verify and peer review and this faster to merge.
With respect to Code moves and CS, I believe we should have a "refactoring
fortnight" where we so the bulk of code move-only refactoring plus CS where
necessary. This is by fat the most disruptive kind of change because it
widely affects other PRs mergeability. We should aim to get most of this
done in one go, so that it's not happening in dribs and drabs over months
and many releases. Once done, it gives everyone a good idea to the overall
new structure and where one can expect to find things in the future. The
idea here is to help orientation and not have to continuously hunt for
where things have moved to.
To be clear, I am strongly suggesting code move-only refactoring PRs not
be mixed with anything else. Same for CS changes. This makes the PRs
extremely easy to vet and thus quick to merge.
Towards this end, maybe there should be an IRC meeting to agree the
initial moves, then someone who has the stomach for it can get on and do it
- during that time, we do not merge anything else. We need to bite the
bullet and break the back out of code moves.
With regards to CS, I think we do need to get CS right, because a
continual dribble of CS changes also makes diffs between releases less easy
to follow. Much of CS checking can be automated by the continuous
integration so authors can get it right easily. It can be just like a
Travis check.
With respect to the 3rd kind of refactoring, we need to set some standards
and goals and aim for some kind of consistency. Refactoring needs to fulfil
certain goals and criterion otherwise contributors will always find a
reason to fiddle over and over forever. Obvious targets here can be things
like proper encapsulation and separation of concerns.
Overall, refactoring should be merged quickly, but only on a schedule so
it doesn't cause major disruption to others.
Obviously the third kind of refactoring more complex and time consuming
and will need to occur over time, but it should happen in defined steps. As
Jeff said, one week a month, or maybe one month a release. In any case,
refactoring changes should be quickly accepted or rejected by the project
maintainer and not left hanging.
Finally, refactoring should *always* be uncontroversial because
essentially functionality is not changing. If functionality changes (e.g.
you try to sneak in a big fix or feature tweak "because it's small") the PR
should be rejected outright. Additionally, if we break down refactoring
into the three kinds stated above, peer review will be much more
straightforward.
On Tue, Sep 15, 2015 at 5:10 AM, Jeff Garzik via bitcoin-dev <
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
The general goal is good. But we must not stray into unfocused
engineering for a non-existent future library user.
The higher priority must be given to having a source code base that
maximizes the collective developers' ability to maintain The Router --
the core bitcoin full node P2P engine.
I recommend time-based bursts of code movement changes. See below;
for example, just submit & merge code movement changes on the first
week of every 2nd month. Code movement changes are easy to create
from scratch once a concrete goal is known. The coding part is
trivial and takes no time.
As we saw in the Linux kernel - battle lessons hard learned - code
movement and refactors have often unseen negative impact on downstream
developers working on more complicated changes that have more positive
impact to our developers and users.
On Bitcoin development release cycles & process
------------------------------------------------------------------
As I've outlined in the past, the Linux kernel maintenance phases
address some of these problems. The merge window into git master
opens for 1 week, a very chaotic week full of merging (and rebasing),
and then the merge window closes. Several weeks follow as the "dust
settles" -- testing, bug fixing, moving in parallel OOB with
not-yet-ready development. Release candidates follow, then the
release, then the cycle repeats.
IMO a merge window approach fixes some of the issues with refactoring,
as well as introduces some useful -developer discipline- into the
development process. Bitcoin Core still needs rapid iteration --
another failing of the current project -- and so something of a more
- 1st week of each month, merge changes. Lots of rebasing during this week.
- remaining days of the month, test, bug fix
- release at end of month
If changes are not ready for merging, then so be it, they wait until
next month's release. Some releases have major features, some
releases are completely boring and offer little of note. That is the
nature of time-based development iteration. It's like dollar cost
averaging, a bit.
And frankly, I would like to close all github pull requests that are
not ready to merge That Week. I'm as guilty of this as any, but that
stuff just languishes. Excluding a certain category of obvious-crap,
pull requests tend to default to a state of either (a) rapid merging,
(b) months-long issues/projects, (c) limbo.
Under a more time-based approach, a better pull request process would be to
* Only open pull requests if it's a bug fix, or the merge window is
open and the change is ready to be merged in the developer's opinion.
* Developers CC bitcoin-dev list to discuss Bitcoin Core-bound projects
* Developers maintain and publish projects via their own git trees
* Pull requests should be closed if unmerged after 7 days, unless it
is an important bug fix etc.
The problem with projects like libconsensus is that they can get
unfocused and open ended. Code movement changes in particular are
cheap to generate. It is low developer cost for the developer to
iterate all the way to the end state, see what that looks like, and
see if people like it. That end state is not something you would
merge all in one go. I would likely stash that tree, and then start
again, seek the most optimal and least disruptive set of refactors,
and generate and merge those into bitcoin/bitcoin.git in a time-based,
paced manner. Announce the pace ahead of time - "cosmetic stuff that
breaks your patches will be merged 1st week of every second month"
To underscore, the higher priority must be given to having a source
code base and disciplined development process that maximizes the
collective developers' ability to maintain The Router that maintains
most of our network.
Modularity, refactoring, cleaning up grotty code generates a deep
seated happiness in many engineers. Field experience however shows
refactoring is a never ending process which sometimes gets in the way
of More Important Work.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
Eric Lombrozo via bitcoin-dev
2015-09-15 16:00:11 UTC
Permalink
I basically agree with what has been said here.

Refactoring efforts should be well-coordinated. Their short-term impact can be quite disruptive, although if done correctly, longer-term they make it even easier for downstream developers to add and merge changes.

By scheduling move-only changes, others can avoid making PRs immediately prior to or during these changes (which ironically involve considerable disruption to PRs while changing nothing for endusers). Furthermore, it would be useful to document the changes in ways that help other developers rebase properly.
Post by Jeff Garzik via bitcoin-dev
Drak,
I would say that the refactoring does actually fulfill some conditions you
- move-only is almost always clearly separated out
- the refactoring is not controversial _in minimis_ - meaning, the
individual pull request is not controversial.
The problem comes with the impact of an unfocused stream of refactors to
key code.
For example, there is much less long term developer impact if
refactoring
were _accelerated_, scheduled to be performed in a one-week sprint.
There
is a lot of breakage, yes, but after that week the average level of
downstream patch breakage is significantly lower. A "rip the band-aid off
quickly rather than slowly" approach.
Post by Btc Drak via bitcoin-dev
I also share a lot of Jeff's concerns about refactoring and have
voiced
Post by Btc Drak via bitcoin-dev
them several times on IRC and in private to Jorge, Wladamir and Greg.
I
Post by Btc Drak via bitcoin-dev
meant to do a write up but never got around to it. Jeff has quite
eloquently stated the various problems. I would like to share my
thoughts
Post by Btc Drak via bitcoin-dev
on the matter because we really do need to come up with a plan on how
this
Post by Btc Drak via bitcoin-dev
issue is dealt with.
Obviously, Bitcoin Core is quite tightly coupled at the moment and
definitely needs extensive modularisation. Such work will inevitably
require lots of bulk code moves and then finer refactoring. However,
it
Post by Btc Drak via bitcoin-dev
requires proper planning because there are lots of effects and
consequences
Post by Btc Drak via bitcoin-dev
for other people contributing to Core and also downstream projects
relying
Post by Btc Drak via bitcoin-dev
1. Refactoring often causes other pull requests to diverge and
require
Post by Btc Drak via bitcoin-dev
rebasing. Continual refactoring can put PRs in "rebase hell" and puts
a big
Post by Btc Drak via bitcoin-dev
stress on contributors (many of whom are part time).
2. Version to version, Bitcoin Core changes significantly in
structure.
Post by Btc Drak via bitcoin-dev
0.9 to 0.10 is unrecognisable. 0.10 to 0.11 is even more so. This
makes
Post by Btc Drak via bitcoin-dev
makes it hard to follow release to release and the net result is less
people upgrade (especially think of miners trying to keep their patch
sets
Post by Btc Drak via bitcoin-dev
working while trying not to disrupt or risk their mining operations).
3. Continual refactoring increases risk: we're human, and mistakes
will
Post by Btc Drak via bitcoin-dev
slip through peer review. This is especially concerning with
consensus
Post by Btc Drak via bitcoin-dev
critical code and this makes it difficult to merge such refactoring
often,
Post by Btc Drak via bitcoin-dev
which of course exacerbates the problem.
The net negative consequence is it is harder to contribute to Core,
harder
Post by Btc Drak via bitcoin-dev
for the Core maintainers to merge and harder for downstream/dependent
projects/implementations to keep up.
Suggested Way Forward
---------------------------------
With the understanding that refactored code by definition must not
change
Post by Btc Drak via bitcoin-dev
1. code moves (e.g. separating concerns into different files);
2. code style;
3. structural optimisation and consolidation (reducing LOC,
separating
Post by Btc Drak via bitcoin-dev
concerns, encapsulation etc).
Code moves(1) and CS(2) are easy to peer review and merge quickly.
The
Post by Btc Drak via bitcoin-dev
third kind(3) requires deeper analysis to ensure that while the code
changed, the behaviour (including any bugs) did not.
We must resist all temptation to fix bugs or tack on minor fixes and
tweaks during refactoring: pull requests should only be refactoring
only,
Post by Btc Drak via bitcoin-dev
with no net change to behaviour. Keeping discipline makes it much
easier to
Post by Btc Drak via bitcoin-dev
verify and peer review and this faster to merge.
With respect to Code moves and CS, I believe we should have a
"refactoring
Post by Btc Drak via bitcoin-dev
fortnight" where we so the bulk of code move-only refactoring plus CS
where
Post by Btc Drak via bitcoin-dev
necessary. This is by fat the most disruptive kind of change because
it
Post by Btc Drak via bitcoin-dev
widely affects other PRs mergeability. We should aim to get most of
this
Post by Btc Drak via bitcoin-dev
done in one go, so that it's not happening in dribs and drabs over
months
Post by Btc Drak via bitcoin-dev
and many releases. Once done, it gives everyone a good idea to the
overall
Post by Btc Drak via bitcoin-dev
new structure and where one can expect to find things in the future.
The
Post by Btc Drak via bitcoin-dev
idea here is to help orientation and not have to continuously hunt
for
Post by Btc Drak via bitcoin-dev
where things have moved to.
To be clear, I am strongly suggesting code move-only refactoring PRs
not
Post by Btc Drak via bitcoin-dev
be mixed with anything else. Same for CS changes. This makes the PRs
extremely easy to vet and thus quick to merge.
Towards this end, maybe there should be an IRC meeting to agree the
initial moves, then someone who has the stomach for it can get on and
do it
Post by Btc Drak via bitcoin-dev
- during that time, we do not merge anything else. We need to bite
the
Post by Btc Drak via bitcoin-dev
bullet and break the back out of code moves.
With regards to CS, I think we do need to get CS right, because a
continual dribble of CS changes also makes diffs between releases
less easy
Post by Btc Drak via bitcoin-dev
to follow. Much of CS checking can be automated by the continuous
integration so authors can get it right easily. It can be just like a
Travis check.
With respect to the 3rd kind of refactoring, we need to set some
standards
Post by Btc Drak via bitcoin-dev
and goals and aim for some kind of consistency. Refactoring needs to
fulfil
Post by Btc Drak via bitcoin-dev
certain goals and criterion otherwise contributors will always find a
reason to fiddle over and over forever. Obvious targets here can be
things
Post by Btc Drak via bitcoin-dev
like proper encapsulation and separation of concerns.
Overall, refactoring should be merged quickly, but only on a schedule
so
Post by Btc Drak via bitcoin-dev
it doesn't cause major disruption to others.
Obviously the third kind of refactoring more complex and time
consuming
Post by Btc Drak via bitcoin-dev
and will need to occur over time, but it should happen in defined
steps. As
Post by Btc Drak via bitcoin-dev
Jeff said, one week a month, or maybe one month a release. In any
case,
Post by Btc Drak via bitcoin-dev
refactoring changes should be quickly accepted or rejected by the
project
Post by Btc Drak via bitcoin-dev
maintainer and not left hanging.
Finally, refactoring should *always* be uncontroversial because
essentially functionality is not changing. If functionality changes
(e.g.
Post by Btc Drak via bitcoin-dev
you try to sneak in a big fix or feature tweak "because it's small")
the PR
Post by Btc Drak via bitcoin-dev
should be rejected outright. Additionally, if we break down
refactoring
Post by Btc Drak via bitcoin-dev
into the three kinds stated above, peer review will be much more
straightforward.
On Tue, Sep 15, 2015 at 5:10 AM, Jeff Garzik via bitcoin-dev <
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is
a
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code
movement
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct,
easy
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple
effects.
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to
each
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for
normal
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for
the
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in
the
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
dark. Where is any sort of plan? I have low standards - a photo of
a
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
whiteboard or youtube clip will do.
The general goal is good. But we must not stray into unfocused
engineering for a non-existent future library user.
The higher priority must be given to having a source code base that
maximizes the collective developers' ability to maintain The Router
--
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
the core bitcoin full node P2P engine.
I recommend time-based bursts of code movement changes. See below;
for example, just submit & merge code movement changes on the first
week of every 2nd month. Code movement changes are easy to create
from scratch once a concrete goal is known. The coding part is
trivial and takes no time.
As we saw in the Linux kernel - battle lessons hard learned - code
movement and refactors have often unseen negative impact on
downstream
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
developers working on more complicated changes that have more
positive
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
impact to our developers and users.
On Bitcoin development release cycles & process
------------------------------------------------------------------
As I've outlined in the past, the Linux kernel maintenance phases
address some of these problems. The merge window into git master
opens for 1 week, a very chaotic week full of merging (and
rebasing),
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
and then the merge window closes. Several weeks follow as the "dust
settles" -- testing, bug fixing, moving in parallel OOB with
not-yet-ready development. Release candidates follow, then the
release, then the cycle repeats.
IMO a merge window approach fixes some of the issues with
refactoring,
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
as well as introduces some useful -developer discipline- into the
development process. Bitcoin Core still needs rapid iteration --
another failing of the current project -- and so something of a more
- 1st week of each month, merge changes. Lots of rebasing during
this
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
week.
- remaining days of the month, test, bug fix
- release at end of month
If changes are not ready for merging, then so be it, they wait until
next month's release. Some releases have major features, some
releases are completely boring and offer little of note. That is
the
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
nature of time-based development iteration. It's like dollar cost
averaging, a bit.
And frankly, I would like to close all github pull requests that are
not ready to merge That Week. I'm as guilty of this as any, but
that
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
stuff just languishes. Excluding a certain category of
obvious-crap,
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
pull requests tend to default to a state of either (a) rapid
merging,
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
(b) months-long issues/projects, (c) limbo.
Under a more time-based approach, a better pull request process
would be
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
to
* Only open pull requests if it's a bug fix, or the merge window is
open and the change is ready to be merged in the developer's
opinion.
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
* Developers CC bitcoin-dev list to discuss Bitcoin Core-bound
projects
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
* Developers maintain and publish projects via their own git trees
* Pull requests should be closed if unmerged after 7 days, unless it
is an important bug fix etc.
The problem with projects like libconsensus is that they can get
unfocused and open ended. Code movement changes in particular are
cheap to generate. It is low developer cost for the developer to
iterate all the way to the end state, see what that looks like, and
see if people like it. That end state is not something you would
merge all in one go. I would likely stash that tree, and then start
again, seek the most optimal and least disruptive set of refactors,
and generate and merge those into bitcoin/bitcoin.git in a
time-based,
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
paced manner. Announce the pace ahead of time - "cosmetic stuff
that
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
breaks your patches will be merged 1st week of every second month"
To underscore, the higher priority must be given to having a source
code base and disciplined development process that maximizes the
collective developers' ability to maintain The Router that maintains
most of our network.
Modularity, refactoring, cleaning up grotty code generates a deep
seated happiness in many engineers. Field experience however shows
refactoring is a never ending process which sometimes gets in the
way
Post by Btc Drak via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
of More Important Work.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
------------------------------------------------------------------------
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Btc Drak via bitcoin-dev
2015-09-15 18:26:50 UTC
Permalink
Post by Jeff Garzik via bitcoin-dev
The problem comes with the impact of an unfocused stream of refactors to
key code.
For example, there is much less long term developer impact if refactoring
were _accelerated_, scheduled to be performed in a one-week sprint. There
is a lot of breakage, yes, but after that week the average level of
downstream patch breakage is significantly lower. A "rip the band-aid off
quickly rather than slowly" approach.
My sentiments exactly...
Peter Todd via bitcoin-dev
2015-09-16 22:29:28 UTC
Permalink
Post by Jeff Garzik via bitcoin-dev
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
Incidentally, it'd help if we got some insight into why those branches
are being maintained; what features are in those branches that Bitcoin
Core doesn't have?

I've run into a number of cases where companies were maintaining forks
of Bitcoin Core unnecessarily, where a different, loosely coupled,
architecture could do what they needed to do without including the new
logic in the codebase itself.
--
'peter'[:-1]@petertodd.org
000000000000000013137b1bd77e352d28fa36309be1c821180eda408bcb745c
Wladimir J. van der Laan via bitcoin-dev
2015-09-18 00:07:20 UTC
Permalink
Post by Peter Todd via bitcoin-dev
I've run into a number of cases where companies were maintaining forks
of Bitcoin Core unnecessarily, where a different, loosely coupled,
architecture could do what they needed to do without including the new
logic in the codebase itself.
This is the same point I have been making to Jeff privately.

Refactors are a means to an end: a more modular, reusable and maintainable codebase. This goal is that new functionality can be plugged in more easily, and rebase work for e.g. functionality built on top can go down, not up, if it just hooks into well-defined interfaces here and there.

Although there has been a lot of progress, bitcoind's design is still too monolithic. To add a more involved feature, like say a new index over the block chain data, code needs to be touched all over the place. This change interacts with all other functionality, potentially breaking the base node functionality - risk for users that do NOT use the functionality. This increases risk and review time.

- *If possible* functionality should be built without changing bitcoind's code at all. An external process should be able to keep up to date with the chain, notice reorgs, and process block data accordingly. If bitcoind's interface does not allow that, or it is too difficult, that is what should be fixed.
- *if not possible* then a change should at least touch the code in as few places as possible, and integrate with e.g. signal notification.

To name an example of it done right, IMO: Monero's 'simplewallet'. It is a command-line utility wallet that communicates with the node software, and remembers where it was in the chain, and processes changes to the chain state since its last invocation when it 'refreshes'.
What is nice is that one can run an arbitary number of simplewallets against one node daemon, and unlike bitcoind's wallet it doesn't need to run as always-on daemon itself. It can be invoked when the user wants to do something with the wallet, or see if there are new transactions.

An index could be implemented entirely externally in a similar way, while still fully handling reorgs.

What one needs for that, I think, is a library that communicate with the node, and which offers functionality abstractly be similar to 'git pull': give me the tree path from my current known tip to the best tip, and supply the block hashes (and block data) along the way.

My long-term vision of bitcoind is a P2P node with validation and blockchain store, with a couple of data sources that can be subscribed to or pulled from.

Wladimir
Eric Lombrozo via bitcoin-dev
2015-09-18 08:42:53 UTC
Permalink
You're aware that my entire stack was built around this model and I've even built a fully fledged desktop GUI, multisig account manager, and servers supporting pull and event subscription atop it, right?
Post by Wladimir J. van der Laan via bitcoin-dev
Post by Peter Todd via bitcoin-dev
I've run into a number of cases where companies were maintaining
forks
Post by Peter Todd via bitcoin-dev
of Bitcoin Core unnecessarily, where a different, loosely coupled,
architecture could do what they needed to do without including the
new
Post by Peter Todd via bitcoin-dev
logic in the codebase itself.
This is the same point I have been making to Jeff privately.
Refactors are a means to an end: a more modular, reusable and
maintainable codebase. This goal is that new functionality can be
plugged in more easily, and rebase work for e.g. functionality built on
top can go down, not up, if it just hooks into well-defined interfaces
here and there.
Although there has been a lot of progress, bitcoind's design is still
too monolithic. To add a more involved feature, like say a new index
over the block chain data, code needs to be touched all over the place.
This change interacts with all other functionality, potentially
breaking the base node functionality - risk for users that do NOT use
the functionality. This increases risk and review time.
- *If possible* functionality should be built without changing
bitcoind's code at all. An external process should be able to keep up
to date with the chain, notice reorgs, and process block data
accordingly. If bitcoind's interface does not allow that, or it is too
difficult, that is what should be fixed.
- *if not possible* then a change should at least touch the code in as
few places as possible, and integrate with e.g. signal notification.
To name an example of it done right, IMO: Monero's 'simplewallet'. It
is a command-line utility wallet that communicates with the node
software, and remembers where it was in the chain, and processes
changes to the chain state since its last invocation when it
'refreshes'.
What is nice is that one can run an arbitary number of simplewallets
against one node daemon, and unlike bitcoind's wallet it doesn't need
to run as always-on daemon itself. It can be invoked when the user
wants to do something with the wallet, or see if there are new
transactions.
An index could be implemented entirely externally in a similar way,
while still fully handling reorgs.
What one needs for that, I think, is a library that communicate with
the node, and which offers functionality abstractly be similar to 'git
pull': give me the tree path from my current known tip to the best tip,
and supply the block hashes (and block data) along the way.
My long-term vision of bitcoind is a P2P node with validation and
blockchain store, with a couple of data sources that can be subscribed
to or pulled from.
Wladimir
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Mike Hearn via bitcoin-dev
2015-09-18 16:22:09 UTC
Permalink
Post by Wladimir J. van der Laan via bitcoin-dev
What one needs for that, I think, is a library that communicate with the
give me the tree path from my current known tip to the best tip, and supply
the block hashes (and block data) along the way.
This is exactly what SPV libraries like bitcoinj do: they know how to build
a block locator, request the blocks forward from the common branch point,
and handle re-orgs onto whatever the current best chain are by downloading
data from a full node.

If your official position is people should all use bitcoinj to do things
like build extra indexes, then great. Send them our way. It already knows
how to calculate a UTXO set indexed by address.
Jorge Timón via bitcoin-dev
2015-09-22 18:12:41 UTC
Permalink
On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
Just because you don't understand the changes proposed it doesn't mean
that they are random.
I may have done a poor job in communicating "my plan for libconsensus"
but I have tried many times and in many ways.
#bitcoin-dev logs show that I have not worked "in the dark" at all, on
the contrary, I've been very tenacious when asking for review and
opinions, to the point that several people (at least @laanwj and
@theuni have complained about their github inboxes being full of
"spam").
This is a relatively recent thread where I describe my plan:
http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009568.html
Not my first attempt on this list.

It is very frustrating that everybody seems to agree that separating
libconsensus is a priority to maximize the number of people that can
safely contribute to the project, but at the same time, nobody thinks
that reviewing the necessary refactors to do so is a priority.
I tried creating big PRs for people to see "the big picture" #5946 but
those were too many commits and nobody wanted to read it. Gavin asked
for an API.
So I tried a smaller step: exposing just VerifyHeader in libconsensus
and leave VerifyTx and VerifyBlock for later #5995
Again, this was "too big" and "a moving target". In the meantime I
always had smaller one-little-step PRs that were part of a longer
branch:

** [8/8] MERGED Consensus
- [X] Consensus: Decouple pow from chainparams #5812 [consensuspow]
- [X] MOVEONLY: Move constants and globals to consensus.h #5696
[consensus_policy0]
- [X] Chainparams: Refactor: Decouple IsSuperMajority from Params()
#5968 [params_consensus]
- [X] Remove redundant getter CChainParams::SubsidyHalvingInterval()
#5996 [params_subsidy]
- [X] Separate CValidationState from main #5669 [consensus]
- [X] Consensus: Decouple ContextualCheckBlockHeader from checkpoints
#5975 [consensus_checkpoints]
- [X] Separate Consensus::CheckTxInputs and GetSpendHeight in
CheckInputs #6061 [consensus_inputs]
- [X] Bugfix: Don't check the genesis block header before accepting it
#6299 [5975-quick-fix]
** [5/5] DELETED
*** DELETED Refactor: Create CCoinsViewEfficient interface for
CCoinsViewCache #5747 [coins]
*** DELETED Chainparams: Explicit Consensus::Params arg in consensus
functions #6024 [params_consensus2]
*** DELETED MOVEONLY: Move most of consensus functions (pre-block)
#6051 [consensus_moveonly] (depends on consensus-blocksize-0.12.99)
*** DELETED Consensus: Refactor: Separate CheckFinalTx from
main::IsFinalTx #6063 [consensus_finaltx]
*** DELETED Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast
into independent function #6009 [consensus_mediantime]
*** DELETED Consensus: Adapt declarations of most obviously consensus
functions #6591 [consensus-params-0.12.99]
*** DELETED Consensus: Move blocksize and related parameters to
consensusparams ...without removing consensus/consensus.h [#6526
alternative] #6625 [consensus-blocksize-0.12.99]

After a while I stop rebasing the longer branches and just maintained
a few small consensus-related PRs at a time.

Now I consolidated 3 of them in

*** REVIEW Optimizations: Consensus: In AcceptToMemoryPool,
ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99]

with the hope that it would be merged relatively fast.
After that it will be much simpler to start talking about potential C
APIs for VerifyHeader, VerifyTx and VerifyBlock; as well as separating
the library to a subtree.

I'm more than happy to answer any questions anyone may have about any
of the PRs or commits, until everybody interested is convinced that
there's nothing random in the proposed changes.
I'm also more than happy to get advice on how to better communicate my
plans and structure my PRs.
Dave Scotese via bitcoin-dev
2015-09-22 23:49:11 UTC
Permalink
If I'm reading this situation correctly, Jeff is basically pointing out
that developers need more links (hooks, rungs, handholds, data points,
whatever you want to call them) so that they can see all the things his
email insinuated are missing (a plan, order, sense, etc.). He didn't say
these things were missing, but that it kind of feels like it from the
10,000 foot view.

If you use Google to search the list, as in <<site:lists.linuxfoundation.org
libconsensus plan>> you DO NOT get the page Jorge gave. He wrote that
page, so he had a good idea what to search for to find it again. I just
want to recommend that when you describe the work you're doing on bitcoin,
imagine several different ways people might try to find this description in
the future and make them work. In other words, Jorge could have put "A
plan for abstracting out libconsensus" in the email where he wrote "Here
are some things that need to happen first..."

Likewise, if Jeff had searched for <<site:lists.linuxfoundation.org
libconsensus plan>> (maybe he did, but he didn't list any results), he may
have found enough clues to see Jorge's overall plan. The "site:" keyword
on Google fascinated me when I discovered it, so I let it inspire this
email :-)

Maybe someone can explain this if I have it wrong: A few people are able to
pull code into Bitcoin/bitcoin. Isn't is possible that those few people
can agree to merge in a lot of refactor-hell PRs for those making the
requests, but postpone them to that one-week-per-month that someone
suggested? The idea of letting that "hell" come in (predictable) waves is
excellent and I was hoping to see some agreement. But I don't know who
those few are, so even if they all wrote "Yeah, we'll do that," I wouldn't
recognize that I got what I wanted.

notplato

On Tue, Sep 22, 2015 at 11:12 AM, Jorge Timón <
Post by Jorge Timón via bitcoin-dev
On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
Just because you don't understand the changes proposed it doesn't mean
that they are random.
I may have done a poor job in communicating "my plan for libconsensus"
but I have tried many times and in many ways.
#bitcoin-dev logs show that I have not worked "in the dark" at all, on
the contrary, I've been very tenacious when asking for review and
@theuni have complained about their github inboxes being full of
"spam").
http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009568.html
Not my first attempt on this list.
It is very frustrating that everybody seems to agree that separating
libconsensus is a priority to maximize the number of people that can
safely contribute to the project, but at the same time, nobody thinks
that reviewing the necessary refactors to do so is a priority.
I tried creating big PRs for people to see "the big picture" #5946 but
those were too many commits and nobody wanted to read it. Gavin asked
for an API.
So I tried a smaller step: exposing just VerifyHeader in libconsensus
and leave VerifyTx and VerifyBlock for later #5995
Again, this was "too big" and "a moving target". In the meantime I
always had smaller one-little-step PRs that were part of a longer
** [8/8] MERGED Consensus
- [X] Consensus: Decouple pow from chainparams #5812 [consensuspow]
- [X] MOVEONLY: Move constants and globals to consensus.h #5696
[consensus_policy0]
- [X] Chainparams: Refactor: Decouple IsSuperMajority from Params()
#5968 [params_consensus]
- [X] Remove redundant getter CChainParams::SubsidyHalvingInterval()
#5996 [params_subsidy]
- [X] Separate CValidationState from main #5669 [consensus]
- [X] Consensus: Decouple ContextualCheckBlockHeader from checkpoints
#5975 [consensus_checkpoints]
- [X] Separate Consensus::CheckTxInputs and GetSpendHeight in
CheckInputs #6061 [consensus_inputs]
- [X] Bugfix: Don't check the genesis block header before accepting it
#6299 [5975-quick-fix]
** [5/5] DELETED
*** DELETED Refactor: Create CCoinsViewEfficient interface for
CCoinsViewCache #5747 [coins]
*** DELETED Chainparams: Explicit Consensus::Params arg in consensus
functions #6024 [params_consensus2]
*** DELETED MOVEONLY: Move most of consensus functions (pre-block)
#6051 [consensus_moveonly] (depends on consensus-blocksize-0.12.99)
*** DELETED Consensus: Refactor: Separate CheckFinalTx from
main::IsFinalTx #6063 [consensus_finaltx]
*** DELETED Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast
into independent function #6009 [consensus_mediantime]
*** DELETED Consensus: Adapt declarations of most obviously consensus
functions #6591 [consensus-params-0.12.99]
*** DELETED Consensus: Move blocksize and related parameters to
consensusparams ...without removing consensus/consensus.h [#6526
alternative] #6625 [consensus-blocksize-0.12.99]
After a while I stop rebasing the longer branches and just maintained
a few small consensus-related PRs at a time.
Now I consolidated 3 of them in
*** REVIEW Optimizations: Consensus: In AcceptToMemoryPool,
ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99]
with the hope that it would be merged relatively fast.
After that it will be much simpler to start talking about potential C
APIs for VerifyHeader, VerifyTx and VerifyBlock; as well as separating
the library to a subtree.
I'm more than happy to answer any questions anyone may have about any
of the PRs or commits, until everybody interested is convinced that
there's nothing random in the proposed changes.
I'm also more than happy to get advice on how to better communicate my
plans and structure my PRs.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
--
I like to provide some work at no charge to prove my value. Do you need a
techie?
I own Litmocracy <http://www.litmocracy.com> and Meme Racing
<http://www.memeracing.net> (in alpha).
I'm the webmaster for The Voluntaryist <http://www.voluntaryist.com> which
now accepts Bitcoin.
I also code for The Dollar Vigilante <http://dollarvigilante.com/>.
"He ought to find it more profitable to play by the rules" - Satoshi
Nakamoto
Jorge Timón via bitcoin-dev
2015-09-23 17:28:06 UTC
Permalink
If I'm reading this situation correctly, Jeff is basically pointing out that
developers need more links (hooks, rungs, handholds, data points, whatever
you want to call them) so that they can see all the things his email
insinuated are missing (a plan, order, sense, etc.). He didn't say these
things were missing, but that it kind of feels like it from the 10,000 foot
view.
If you use Google to search the list, as in <<site:lists.linuxfoundation.org
libconsensus plan>> you DO NOT get the page Jorge gave. He wrote that page,
so he had a good idea what to search for to find it again. I just want to
recommend that when you describe the work you're doing on bitcoin, imagine
several different ways people might try to find this description in the
future and make them work. In other words, Jorge could have put "A plan for
abstracting out libconsensus" in the email where he wrote "Here are some
things that need to happen first..."
Likewise, if Jeff had searched for <<site:lists.linuxfoundation.org
libconsensus plan>> (maybe he did, but he didn't list any results), he may
have found enough clues to see Jorge's overall plan. The "site:" keyword on
Google fascinated me when I discovered it, so I let it inspire this email
:-)
My fault: https://github.com/bitcoin/bitcoin/issues/6714
On Tue, Sep 22, 2015 at 11:12 AM, Jorge Timón
Post by Jorge Timón via bitcoin-dev
On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
Just because you don't understand the changes proposed it doesn't mean
that they are random.
I may have done a poor job in communicating "my plan for libconsensus"
but I have tried many times and in many ways.
#bitcoin-dev logs show that I have not worked "in the dark" at all, on
the contrary, I've been very tenacious when asking for review and
@theuni have complained about their github inboxes being full of
"spam").
http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009568.html
Not my first attempt on this list.
It is very frustrating that everybody seems to agree that separating
libconsensus is a priority to maximize the number of people that can
safely contribute to the project, but at the same time, nobody thinks
that reviewing the necessary refactors to do so is a priority.
I tried creating big PRs for people to see "the big picture" #5946 but
those were too many commits and nobody wanted to read it. Gavin asked
for an API.
So I tried a smaller step: exposing just VerifyHeader in libconsensus
and leave VerifyTx and VerifyBlock for later #5995
Again, this was "too big" and "a moving target". In the meantime I
always had smaller one-little-step PRs that were part of a longer
** [8/8] MERGED Consensus
- [X] Consensus: Decouple pow from chainparams #5812 [consensuspow]
- [X] MOVEONLY: Move constants and globals to consensus.h #5696
[consensus_policy0]
- [X] Chainparams: Refactor: Decouple IsSuperMajority from Params()
#5968 [params_consensus]
- [X] Remove redundant getter CChainParams::SubsidyHalvingInterval()
#5996 [params_subsidy]
- [X] Separate CValidationState from main #5669 [consensus]
- [X] Consensus: Decouple ContextualCheckBlockHeader from checkpoints
#5975 [consensus_checkpoints]
- [X] Separate Consensus::CheckTxInputs and GetSpendHeight in
CheckInputs #6061 [consensus_inputs]
- [X] Bugfix: Don't check the genesis block header before accepting it
#6299 [5975-quick-fix]
** [5/5] DELETED
*** DELETED Refactor: Create CCoinsViewEfficient interface for
CCoinsViewCache #5747 [coins]
*** DELETED Chainparams: Explicit Consensus::Params arg in consensus
functions #6024 [params_consensus2]
*** DELETED MOVEONLY: Move most of consensus functions (pre-block)
#6051 [consensus_moveonly] (depends on consensus-blocksize-0.12.99)
*** DELETED Consensus: Refactor: Separate CheckFinalTx from
main::IsFinalTx #6063 [consensus_finaltx]
*** DELETED Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast
into independent function #6009 [consensus_mediantime]
*** DELETED Consensus: Adapt declarations of most obviously consensus
functions #6591 [consensus-params-0.12.99]
*** DELETED Consensus: Move blocksize and related parameters to
consensusparams ...without removing consensus/consensus.h [#6526
alternative] #6625 [consensus-blocksize-0.12.99]
After a while I stop rebasing the longer branches and just maintained
a few small consensus-related PRs at a time.
Now I consolidated 3 of them in
*** REVIEW Optimizations: Consensus: In AcceptToMemoryPool,
ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99]
with the hope that it would be merged relatively fast.
After that it will be much simpler to start talking about potential C
APIs for VerifyHeader, VerifyTx and VerifyBlock; as well as separating
the library to a subtree.
I'm more than happy to answer any questions anyone may have about any
of the PRs or commits, until everybody interested is convinced that
there's nothing random in the proposed changes.
I'm also more than happy to get advice on how to better communicate my
plans and structure my PRs.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
--
I like to provide some work at no charge to prove my value. Do you need a
techie?
I own Litmocracy and Meme Racing (in alpha).
I'm the webmaster for The Voluntaryist which now accepts Bitcoin.
I also code for The Dollar Vigilante.
"He ought to find it more profitable to play by the rules" - Satoshi
Nakamoto
Jeff Garzik via bitcoin-dev
2015-09-29 13:04:09 UTC
Permalink
There seemed to be some agreement on IRC - after a bit of haranguing by
myself :) -- that large refactors should (a) occur over a small window of
time and (b) have a written plan beforehand.
Post by Dave Scotese via bitcoin-dev
If I'm reading this situation correctly, Jeff is basically pointing out
that developers need more links (hooks, rungs, handholds, data points,
whatever you want to call them) so that they can see all the things his
email insinuated are missing (a plan, order, sense, etc.). He didn't say
these things were missing, but that it kind of feels like it from the
10,000 foot view.
lists.linuxfoundation.org libconsensus plan>> you DO NOT get the page
Jorge gave. He wrote that page, so he had a good idea what to search for
to find it again. I just want to recommend that when you describe the work
you're doing on bitcoin, imagine several different ways people might try to
find this description in the future and make them work. In other words,
Jorge could have put "A plan for abstracting out libconsensus" in the email
where he wrote "Here are some things that need to happen first..."
Likewise, if Jeff had searched for <<site:lists.linuxfoundation.org
libconsensus plan>> (maybe he did, but he didn't list any results), he may
have found enough clues to see Jorge's overall plan. The "site:" keyword
on Google fascinated me when I discovered it, so I let it inspire this
email :-)
Maybe someone can explain this if I have it wrong: A few people are able
to pull code into Bitcoin/bitcoin. Isn't is possible that those few people
can agree to merge in a lot of refactor-hell PRs for those making the
requests, but postpone them to that one-week-per-month that someone
suggested? The idea of letting that "hell" come in (predictable) waves is
excellent and I was hoping to see some agreement. But I don't know who
those few are, so even if they all wrote "Yeah, we'll do that," I wouldn't
recognize that I got what I wanted.
notplato
On Tue, Sep 22, 2015 at 11:12 AM, Jorge Timón <
Post by Jorge Timón via bitcoin-dev
On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
Just because you don't understand the changes proposed it doesn't mean
that they are random.
I may have done a poor job in communicating "my plan for libconsensus"
but I have tried many times and in many ways.
#bitcoin-dev logs show that I have not worked "in the dark" at all, on
the contrary, I've been very tenacious when asking for review and
@theuni have complained about their github inboxes being full of
"spam").
http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009568.html
Not my first attempt on this list.
It is very frustrating that everybody seems to agree that separating
libconsensus is a priority to maximize the number of people that can
safely contribute to the project, but at the same time, nobody thinks
that reviewing the necessary refactors to do so is a priority.
I tried creating big PRs for people to see "the big picture" #5946 but
those were too many commits and nobody wanted to read it. Gavin asked
for an API.
So I tried a smaller step: exposing just VerifyHeader in libconsensus
and leave VerifyTx and VerifyBlock for later #5995
Again, this was "too big" and "a moving target". In the meantime I
always had smaller one-little-step PRs that were part of a longer
** [8/8] MERGED Consensus
- [X] Consensus: Decouple pow from chainparams #5812 [consensuspow]
- [X] MOVEONLY: Move constants and globals to consensus.h #5696
[consensus_policy0]
- [X] Chainparams: Refactor: Decouple IsSuperMajority from Params()
#5968 [params_consensus]
- [X] Remove redundant getter CChainParams::SubsidyHalvingInterval()
#5996 [params_subsidy]
- [X] Separate CValidationState from main #5669 [consensus]
- [X] Consensus: Decouple ContextualCheckBlockHeader from checkpoints
#5975 [consensus_checkpoints]
- [X] Separate Consensus::CheckTxInputs and GetSpendHeight in
CheckInputs #6061 [consensus_inputs]
- [X] Bugfix: Don't check the genesis block header before accepting it
#6299 [5975-quick-fix]
** [5/5] DELETED
*** DELETED Refactor: Create CCoinsViewEfficient interface for
CCoinsViewCache #5747 [coins]
*** DELETED Chainparams: Explicit Consensus::Params arg in consensus
functions #6024 [params_consensus2]
*** DELETED MOVEONLY: Move most of consensus functions (pre-block)
#6051 [consensus_moveonly] (depends on consensus-blocksize-0.12.99)
*** DELETED Consensus: Refactor: Separate CheckFinalTx from
main::IsFinalTx #6063 [consensus_finaltx]
*** DELETED Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast
into independent function #6009 [consensus_mediantime]
*** DELETED Consensus: Adapt declarations of most obviously consensus
functions #6591 [consensus-params-0.12.99]
*** DELETED Consensus: Move blocksize and related parameters to
consensusparams ...without removing consensus/consensus.h [#6526
alternative] #6625 [consensus-blocksize-0.12.99]
After a while I stop rebasing the longer branches and just maintained
a few small consensus-related PRs at a time.
Now I consolidated 3 of them in
*** REVIEW Optimizations: Consensus: In AcceptToMemoryPool,
ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99]
with the hope that it would be merged relatively fast.
After that it will be much simpler to start talking about potential C
APIs for VerifyHeader, VerifyTx and VerifyBlock; as well as separating
the library to a subtree.
I'm more than happy to answer any questions anyone may have about any
of the PRs or commits, until everybody interested is convinced that
there's nothing random in the proposed changes.
I'm also more than happy to get advice on how to better communicate my
plans and structure my PRs.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
--
I like to provide some work at no charge to prove my value. Do you need a
techie?
I own Litmocracy <http://www.litmocracy.com> and Meme Racing
<http://www.memeracing.net> (in alpha).
I'm the webmaster for The Voluntaryist <http://www.voluntaryist.com>
which now accepts Bitcoin.
I also code for The Dollar Vigilante <http://dollarvigilante.com/>.
"He ought to find it more profitable to play by the rules" - Satoshi
Nakamoto
Jorge Timón via bitcoin-dev
2015-09-23 16:58:16 UTC
Permalink
+ Who should use libconsensus
Separating the consensus code is extremely important for less risky
and wider contributions regardless of what is exposed.
But once a complete libconsensus is exposed, alternative
implementations should use it (SPV implementations may not use all of
it though) and Bitcoin Core should eventually use it through its API
as well.
+ What functionality it will provide, and what it won't
It will provide full consensus validation (verification) for the
following structures:

- Script (done, VerifyScript is already exposed)
- Block Headers
- Transactions
- Blocks (including headers and transactions)

The user of the library has to manage storage by itself. This library
will be stateless (apart from libsecp256k1's context) and won't
provide storage.
This library won't tell you which is the longest chain, the highest
level function is VerifyBlock() that just tells you whether a block is
valid or not.
+ How the API works (is it C++ ? C ? Is it stateless ? How is information
sent to/from -- classes ? structs ? serialized data structures ? Are there
callbacks ? How are errors returned ?)
Like the existing libconsensus, a complete libconsensus will have a C API.
The concrete API of each function is to be determined. The exact
concrete way to expose CCoinsViewCache and CBlockIndex (which are not
stateless) will require some discussion.
My preference is using function pointers combined with structs but
there's several possibilities there.
Once the code is separated and the rest of the undesired dependencies
are eliminated, people will be able to propose concrete final APIs
with a few commits.
+ What functions are in the API ?
At the very least:

- VerifyScript
- VerifyHeader
- VerifyTx
- VerifyBlock

To allow users of the library to intertwine policy or DoS checks with
the full verification of a structure (like Bitcoin core does today), I
would also expose at least:

- CheckTransaction/Consensus::CheckTx
- Consensus::CheckTxInputs
- Consensus::CheckTxInputsScripts (doesn't exist yet in master)
- CheckBlockHeader
- ContextualCheckBlockHeader
- CheckBlock
- ContextualCheckBlock
Nobody has time to wade through pull requests to try to figure all that out.
Nobody has the time to review a PR with the many commits necessary to
propose a final independently buildable and complete C API.
This is a work in progress and there's more people participating, not just me.
There's many possible roads that lead to Rome, but let's not allow
perfection be the enemy of walking the very first step.
Can we at least agree on most of the functions that are clearly
consensus critical and separate those so it's easy to build them
separately from main.cpp ?
Can we agree on some of the dependencies that are obviously undesired
and relatively easy to remove?
On Tue, Sep 22, 2015 at 2:12 PM, Jorge Timón
Post by Jorge Timón via bitcoin-dev
On Tue, Sep 15, 2015 at 6:10 AM, Jeff Garzik via bitcoin-dev
Post by Jeff Garzik via bitcoin-dev
[collating a private mail and a github issue comment, moving it to a
better forum]
On libconsensus
---------------
In general there exists the reasonable goal to move consensus state
and code to a specific, separate lib.
To someone not closely reviewing the seemingly endless stream of
libconsensus refactoring PRs, the 10,000 foot view is that there is a
rather random stream of refactors that proceed in fits and starts
without apparent plan or end other than a one sentence "isolate
consensus state and code" summary.
I am hoping that
* There is some plan
* We will not see a five year stream of random consensus code movement
patches causing lots of downstream developer headaches.
I read every code change in every pull request that comes into
* consensus code movement changes - too big, too chaotic, too
frequent, too unfocused, laziness guarantees others will inevitably
ACK it without me.
* some non-code changes (docs)
* ignore 80% of the Qt changes
As with any sort of refactoring, they are easy to prove correct, easy
to reason, and therefore quick and easy to ACK and merge.
Refactors however have a very real negative impact.
bitcoin/bitcoin.git is not only the source tree in the universe.
Software engineers at home, at startups, and at major companies are
maintaining branches of their own.
It is very very easy to fall into a trap where a project is merging
lots of cosmetic changes and not seeing the downstream ripple effects.
Several people complained to me at the conference about all the code
movement changes breaking their own work, causing them to stay on
older versions of bitcoin due to the effort required to rebase to each
new release version - and I share those complaints.
Complex code changes with longer development cycles than simple code
movement patches keep breaking. It is very frustrating, and causes
- Trying to push non-trivial changes upstream is difficult, for normal
and reasonable reasons (big important changes need review etc.).
- Maintaining non-trivial changes out of tree is also painful, for the
aforementioned reasons.
Reasonable work languishes in constant-rebase hell, and incentivizes
against keeping up with the latest tree.
Aside from the refactor, libconsensus appears to be engineering in the
dark. Where is any sort of plan? I have low standards - a photo of a
whiteboard or youtube clip will do.
Just because you don't understand the changes proposed it doesn't mean
that they are random.
I may have done a poor job in communicating "my plan for libconsensus"
but I have tried many times and in many ways.
#bitcoin-dev logs show that I have not worked "in the dark" at all, on
the contrary, I've been very tenacious when asking for review and
@theuni have complained about their github inboxes being full of
"spam").
http://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-July/009568.html
Not my first attempt on this list.
It is very frustrating that everybody seems to agree that separating
libconsensus is a priority to maximize the number of people that can
safely contribute to the project, but at the same time, nobody thinks
that reviewing the necessary refactors to do so is a priority.
I tried creating big PRs for people to see "the big picture" #5946 but
those were too many commits and nobody wanted to read it. Gavin asked
for an API.
So I tried a smaller step: exposing just VerifyHeader in libconsensus
and leave VerifyTx and VerifyBlock for later #5995
Again, this was "too big" and "a moving target". In the meantime I
always had smaller one-little-step PRs that were part of a longer
** [8/8] MERGED Consensus
- [X] Consensus: Decouple pow from chainparams #5812 [consensuspow]
- [X] MOVEONLY: Move constants and globals to consensus.h #5696
[consensus_policy0]
- [X] Chainparams: Refactor: Decouple IsSuperMajority from Params()
#5968 [params_consensus]
- [X] Remove redundant getter CChainParams::SubsidyHalvingInterval()
#5996 [params_subsidy]
- [X] Separate CValidationState from main #5669 [consensus]
- [X] Consensus: Decouple ContextualCheckBlockHeader from checkpoints
#5975 [consensus_checkpoints]
- [X] Separate Consensus::CheckTxInputs and GetSpendHeight in
CheckInputs #6061 [consensus_inputs]
- [X] Bugfix: Don't check the genesis block header before accepting it
#6299 [5975-quick-fix]
** [5/5] DELETED
*** DELETED Refactor: Create CCoinsViewEfficient interface for
CCoinsViewCache #5747 [coins]
*** DELETED Chainparams: Explicit Consensus::Params arg in consensus
functions #6024 [params_consensus2]
*** DELETED MOVEONLY: Move most of consensus functions (pre-block)
#6051 [consensus_moveonly] (depends on consensus-blocksize-0.12.99)
*** DELETED Consensus: Refactor: Separate CheckFinalTx from
main::IsFinalTx #6063 [consensus_finaltx]
*** DELETED Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast
into independent function #6009 [consensus_mediantime]
*** DELETED Consensus: Adapt declarations of most obviously consensus
functions #6591 [consensus-params-0.12.99]
*** DELETED Consensus: Move blocksize and related parameters to
consensusparams ...without removing consensus/consensus.h [#6526
alternative] #6625 [consensus-blocksize-0.12.99]
After a while I stop rebasing the longer branches and just maintained
a few small consensus-related PRs at a time.
Now I consolidated 3 of them in
*** REVIEW Optimizations: Consensus: In AcceptToMemoryPool,
ConnectBlock, and CreateNewBlock #6445 [consensus-txinputs-0.12.99]
with the hope that it would be merged relatively fast.
After that it will be much simpler to start talking about potential C
APIs for VerifyHeader, VerifyTx and VerifyBlock; as well as separating
the library to a subtree.
I'm more than happy to answer any questions anyone may have about any
of the PRs or commits, until everybody interested is convinced that
there's nothing random in the proposed changes.
I'm also more than happy to get advice on how to better communicate my
plans and structure my PRs.
_______________________________________________
bitcoin-dev mailing list
https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
--
--
Gavin Andresen
Loading...