Page MenuHomePhabricator

Turn InvRequestTracker into a template class
ClosedPublic

Authored by Fabien on Thu, May 20, 19:32.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC44ba9c229f89: Turn InvRequestTracker into a template class
Summary

This makes it possible to use it for tracking proofs and not only
transactions. Only uint256 or subclasses are supported as an InvId
due to the copy constructor.

Next steps are to update the names of the files, variable, comments and
public interfaces from tx to inv.

Depends on D9567.

Test Plan
ninja all check-all
./test/functional/test_runner.py p2p_tx_download
ninja bitcoin-fuzzers

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Thu, May 20, 19:32
deadalnix requested changes to this revision.Thu, May 20, 22:36
deadalnix added a subscriber: deadalnix.

Not really requesting changes, but back on your queue.

src/invrequest.h
487 ↗(On Diff #28560)

What is this?

I'm not saying it's wrong, but if you could point me to an example of what that achieve, that would be fantastic.

This revision now requires changes to proceed.Thu, May 20, 22:36
Mengerian added a task: Restricted Maniphest Task.Fri, May 21, 17:09
deadalnix requested changes to this revision.Wed, May 26, 15:39
deadalnix added inline comments.
src/txrequest.h
162 ↗(On Diff #28569)

template constraints are called concepts, no?

336 ↗(On Diff #28569)

All of this can go inline, there is no point declaring it this way.

This revision now requires changes to proceed.Wed, May 26, 15:39
src/txrequest.h
162 ↗(On Diff #28569)

Apparently a concept is a set of contraints, so it's both :)
https://en.cppreference.com/w/cpp/language/constraints

Inline the templatized wrapper definitions

deadalnix added inline comments.
src/txrequest.h
262 ↗(On Diff #28620)

It's be better to avoid a copy here.

This revision is now accepted and ready to land.Wed, May 26, 21:35
This revision was automatically updated to reflect the committed changes.