Main topics

Shared_ptr

Priority removal

Account removal

Auxiliary block requests

Shared_ptr

background

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.

meeting conclusion

Priority removal

background

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.

meeting conclusion

Change default to disable priority relay.

Rehash with Luke-Jr, as he’s the main proponent of keeping priority but not present at meeting

Account removal

background

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.

Earlier this year Wumpus opened a pull request to introduce a ‘label’ API for the wallet (previously mentioned in the 2016-07-14 meeting). It still hasn’t received much review.

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.

meeting conclusion

Review #7729 (rpc: introduce ‘label’ API for wallet)

ping dooglus for user feedback

Auxiliary block requests

background

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.

Comic relief

morcos Lets talk about potential for account or priority removal (2 separate topics) jonasschnelli #topic account or priority removal jonasschnelli #topic "account removal" gmaxwell "Bitcoin developers oppose accountability." jtimon can't we replace accounts with labels all at once? jtimon it's not like we haven't been warning against the use of accounts for ages instagibbs jtimon, at some point I don't think people believe the deprecated warning instagibbs we should put scary ascii art :) gmaxwell okay, I think we should take the proposed action of everyone reading and commenting on 7729 and move to another subject. gmaxwell Or otherwise, we could instead engage in the age old art of completely uninformed combat. gmaxwell "I haven't read 7729 but I oppose any change that causes blindness in small children!" petertodd gmaxwell: I didn't read your last comment, but ACK jonasschnelli If there are no other topic, we could talk about "auxiliary block requests" if some are interested in it? jtimon what is that? gmaxwell jonasschnelli: will that cause blindness in small children?

Participants

Disclaimer

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.