Make feebumper class stateless #10600

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

ryanofsky commented Jun 15, 2017

Make feebumper methods static and remove stored state in the class.

Having the results of feebumper calls persist in an object makes process
separation between Qt and wallet awkward, because it means the feebumper object
either has to be serialized back and forth between Qt and wallet processes
between fee bump calls, or that the feebumper object needs to stay alive in the
wallet process with an object reference passed back to Qt. It's simpler just to
have fee bumper calls return their results immediately instead of storing them
in an object with an extended lifetime.

In addition to making feebumper methods static, also:

  • Move LOCK calls from Qt code to feebumper
  • Move TransactionCanBeBumped implementation from Qt code to feebumper
  • Rename CFeeBumper class to FeeBumper (every CFeeBumper reference had to be
    updated in this PR anyway so this doesn't increase the size of the diff)

This change was originally part of #10244

fanquake added the Wallet label Jun 15, 2017

Contributor

dcousens commented Jun 16, 2017 edited

Why bother with the class?
Practically a namespace at this point?

Contributor

ryanofsky commented Jun 16, 2017

Why bother with the class?
Practically a namespace at this point?

I don't see a problem with using a class as a namespace, but happy to change if you think it is in bad taste and have a different suggestion. Note that the files are called feebumper.h/cpp so if you want to replace the class with something else, it might involve file renames. This PR is really more concerned with simplifying Qt code and moving code that doesn't belong there out.

@ryanofsky ryanofsky Make feebumper class stateless
Make feebumper methods static and remove stored state in the class.

Having the results of feebumper calls persist in an object makes process
separation between Qt and wallet awkward, because it means the feebumper object
either has to be serialized back and forth between Qt and wallet processes
between fee bump calls, or that the feebumper object needs to stay alive in the
wallet process with an object reference passed back to Qt. It's simpler just to
have fee bumper calls return their results immediately instead of storing them
in an object with an extended lifetime.

In addition to making feebumper methods static, also:

- Move LOCK calls from Qt code to feebumper
- Move TransactionCanBeBumped implementation from Qt code to feebumper
- Rename CFeeBumper class to FeeBumper (every CFeeBumper reference had to be
  updated in this PR anyway so this doesn't increase the size of the diff)

This change was originally part of #10244
b937494
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment