IRC meeting summary for 2016-11-17
- Priority removal
- Account removal
- Auxiliary block requests
In many places we’ve started using ‘shared_ptr’ instead of the object itself, this way it can be shared between many data structures without making copies of it.
Sipa has a series of 3 pull requests to introduce shared_ptr in more places, namely #9125 (Make CBlock a vector of shared_ptr of CTransactions), #8580(Make CTransaction actually immutable) and #8589 (Inline CTxInWitness inside CTxIn (on top of #8580)). The first being a necessary refactor for the ones that follow and a performance improvement of 3-4%. The second may be more controversial as it affects the wallet code significantly. Wumpus thinks the old behavior of CwalletTx inheriting from CTransaction is a good example of abuse of inheritance.
- Review #9125
The priority system used to assign transactions with a priority based on age, size, and number of inputs, which made some transactions free. This has a large codebase and efforts have been made to remove the system as it can’t be expected from miners to keep including 0-fee transactions.
Morcos notes the priority code doesn’t serve any function anymore, perhaps a target can be set to remove all the priority code for 0.15. Luke-Jr might disagree with this, although he’s not present at the meeting. There’s a lot of stuff merged on the way for priority already.
Gmaxwell agrees and thinks any desire to keep priority around could be answered by intelligent use of fee delta.
There’s 4 components to “removing priority”: rpc, estimation, mining and relay. Estimation has already been removed. Gmaxwell would like to see relay removed as it currently causes bandwidth wasting as it’s relaying around transactions that won’t get mined.
- Change default to disable priority relay.
- Rehash with Luke-Jr, as he’s the main proponent of keeping priority but not present at meeting
Currently bitcoin-core uses a system of accounts. Third parties using this can have multiple problems with this, and it’s been the consensus for a long time the system should be removed and replaced by a ‘label’ system.
Labels should be introduced first for a release or two before removing accounts as some people still depend on the accounts system, but use it as labels.
Wumpus mentions there should be a protection against users using both account and label API, as this might cause problems.
Instagibbs wonders if there’s anyone talking to devs that still uses accounts. MarcoFalke thinks Dooglus uses it, but his use case would be covered by the new label API.
- Review #7729 (rpc: introduce ‘label’ API for wallet)
- ping dooglus for user feedback
Auxiliary block requests
Jonasschnelli opened a pull request to introduce auxiliary block requests (previously “Out-of-band Block Requests”). This function allows to request blocks, if available on disk, and if not will be downloaded and prioritized over normal IBD downloads. This change is needed to run the SPV wallet, talked about in last weeks meeting.
Multiple developers find it hard to grasp and wonder if there’s a more general description of the high level design and how it all would work. Jonasschnelli explains: you ask your node, give me blocks “D, F, G”, node downloads blocks “D, F, G” and pass it through the signals with validate=false”
Sipa likes the overall concept but thinks the implementation will need to change with the ongoing network refactors.
BlueMatt wonders if the block should really be stored or if we can just pass it to the wallet. If it doesn’t store the block, it’ll need to be downloaded twice in hybrid mode, so it should at least get a chance to store it. BlueMatt would like to see the p2p logic decide whether to send the block to ProcessNewBlock or not.
|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.