IRC meeting summary for 2017-01-26
- Coding style
- Bug-fixing for 0.14
- are we charging adequately for relay?
Stylistic consistency in the code has some benefits:
- it aids newcomers in their contributions because it is easier for them to make sure their work is okay on styleistic grounds.
- It eases review because the uniformity creates better expectations, however reformating makes looking at the history harder, which harms review.
- Good style choices have, at times, been shown to lower defect rates in software, but there is not a universal opinion on what choices are good.
We’re currently not asking people to stick to a particular coding style, which leads to people wondering which style to use where. The current advice to mimick the surrounding code doesn’t actually help in making the codebase converge.
If we all use clang-format on every patch we will converge eventually. Clang-format-diff.py is a tool that does this automatically.
Jonasschnelli once proposed to do a CI check, outside of Travis, to check for clang style, but everyone was against it. BlueMatt is opposed to a CI check, but in favor of a bot which auto-opens a PR which fixes clang style on recently broken PRs. Wumpus doesn’t want any more delay on PRs just because of style issues.
Commits that are move-only should probably not change style as it would otherwise be harder to review whether it’s really move-only.
Gmaxwell’s experience is that small things like coding style nits improve moral in development teams. It’s an opportunity to help each other which is very easy and clear and not a “please totally redesign your patch”.
- Be able to point out style issues in PR’s that conflict with the style guide.
- Use Clang-format-diff.py before submitting a patch whenever possible.
- Don’t change style in move-only commits
Bug-fixing for 0.14
Bitcoin Core 0.14 is scheduled to be released around March 2017. Open pull request aimed for 0.14 are tagged with a 0.14 tag.
Morcos notes his #9615(Wallet incremental fee) needs a 0.14 tag and sdaftuar has one or two minor fixups coming. #9615 splits the default bump-value of the wallet and the relay default incremental fee, which is needed to prevent problems with bumpfee if there’s ever an increase in incrementalRelayFee.
Open PR’s for 0.14 are currently: #9638 #9626 #9622 #9609 #9589 #9108
In 0.14 bumpfee will be released. Gmaxwell proposes to take a look at what GreenAddress and Electrum are doing, as they both have bumpfee in production, to see if they’ve caught anything we’ve missed. He volunteers to check GreenAddress.
- Tag #9615(Wallet incremental fee)
are we charging adequately for relay?
Currently ‘incrementalRelayFee’, which sets the minimum feerate increase for mempool limiting or opt-in replace by fee replacement is set to 1 Satoshi per byte.
Morcos thinks this is too low and the “cost” of the network to relay a transaction around is above 1 Satoshi/byte and there’s insufficient benefit on having that much precision to bumpfee and mempool limiting.
When hearing petertodd talk about how he presses bumpfee in a loop, Morcos realised we are allowing too much relay for 1 transaction being mined. There’s 2 ways to improve this: Raise ‘incrementalRelayFee’, and make it so the behavior of our own code doesn’t cause this ridiculous relay iteration by default if people want to do periodic bumping.
Gmaxwell thinks increasing the limit would cut off transactions that would otherwise confirm in a not crazy amount of time, therefor the relay cost isn’t too high. BlueMatt adds he got a 0.1 Satoshi/byte transaction confirm last weekend pretty easily and thinks it’s premature to discuss bumping it.
Gmaxwell also notes we have some hangover legacy of miners having turned up minRelayTxFee before there was mempool limiting and before createNewBlock was fast. So it may be prudent to first rename the arguments to cause people to reconsider or go back to the default before concluding 1 Satoshi/byte will not confirm. Additionally so as segwit may put the fee behavior back in a disfunctional state.
- Announce minRelayTxFee will be renamed and encourage people to remove it in the release notes.
|wumpus||Wladimir van der Laan|
This summary was compiled without input from any of the participants in the discussion, so any errors are the fault of the summary author and not the discussion participants.