Policy: Move CTxOut::IsDust() to policy.o #5114

pull jtimon wants to merge 2 commits into bitcoin:master from jtimon:dust changing 24 files +122 −94

jtimon commented at 7:32 AM on October 21, 2014: contributor

Built on top of #5100. Decouples CTxOut from CFeeRate. Related to #5071 , now IsDust can become a method of CNodePolicy.

in src/chainparams.h:None in 75c9668bed outdated

   5 | @@ -6,8 +6,8 @@
   6 |  #ifndef BITCOIN_CHAIN_PARAMS_H
   7 |  #define BITCOIN_CHAIN_PARAMS_H
   8 |  
   9 | -#include "core.h"
  10 |  #include "chainparamsbase.h"
  11 | +#include "core/block.h"

Diapolo commented at 10:32 AM on October 21, 2014:

Nit: Should be below checkpoints.

in src/compressor.cpp:None in 75c9668bed outdated
```
 142 | +    if (e < 9) {
 143 | +        int d = (n % 10);
 144 | +        assert(d >= 1 && d <= 9);
 145 | +        n /= 10;
 146 | +        return 1 + (n * 9 + d - 1) * 10 + e;
 147 | +    } else {
```
Diapolo commented at 10:34 AM on October 21, 2014:

This else could just be removed as it's the last return anyway.

jtimon commented at 7:43 PM on October 21, 2014:

But it's not something I want to change in a MOVEONLY commit.

in src/core/transaction.cpp:None in 75c9668bed outdated

  10 | +std::string COutPoint::ToString() const
  11 | +{
  12 | +    return strprintf("COutPoint(%s, %u)", hash.ToString().substr(0, 10), n);
  13 | +}
  14 | +
  15 | +CTxIn::CTxIn(COutPoint prevoutIn, CScript scriptSigIn, uint32_t nSequenceIn)

Diapolo commented at 10:38 AM on October 21, 2014:

Just wondering, why arent't these references?

jtimon commented at 7:43 PM on October 21, 2014:

Again, not something I want to change in a MOVEONLY commit.

in src/init.cpp:None in 75c9668bed outdated
```
   8 | @@ -9,6 +9,7 @@
   9 |  
  10 |  #include "init.h"
  11 |  
  12 | +#include "amount.h"
```
Diapolo commented at 10:39 AM on October 21, 2014:

Nit: Should be below addrman.
jtimon force-pushed on Oct 21, 2014
laanwj added the label Improvement on Oct 22, 2014
jtimon force-pushed on Oct 23, 2014
jtimon force-pushed on Oct 27, 2014
jtimon force-pushed on Oct 28, 2014
jtimon commented at 1:31 PM on October 28, 2014: contributor

Rebased to make it more readable.
sipa commented at 3:37 PM on October 29, 2014: member

If you're moving it, can you move it to main instead? It really doesn't belong in core..
jtimon force-pushed on Oct 30, 2014
jtimon force-pushed on Oct 30, 2014
jtimon commented at 12:28 PM on October 30, 2014: contributor

Moved it to policy.o, where I think it belongs.
sipa commented at 1:34 PM on November 4, 2014: member

utACK
jtimon commented at 10:44 AM on November 5, 2014: contributor

We have to take into account that this will conflict with #5071.
jtimon force-pushed on Dec 27, 2014
jtimon commented at 5:37 PM on December 27, 2014: contributor

Needed rebase (was including core/transaction instead of privitives/transaction).
petertodd commented at 10:12 AM on December 28, 2014: contributor

utACK
jtimon force-pushed on Dec 29, 2014
jtimon commented at 2:12 PM on December 29, 2014: contributor

Added a couple of potential improvements to comment on (moving global minRelayTxFee from main to policy and hiding it for IsDust() calls). Also it needed a small include rebase.
jtimon force-pushed on Dec 29, 2014
jtimon force-pushed on Dec 29, 2014
petertodd commented at 4:20 AM on January 3, 2015: contributor

utACK
jtimon force-pushed on Jan 3, 2015
jtimon renamed this:
~~Turn CTxOut::IsDust() into an independent function~~
Move CTxOut::IsDust() to policy.o
on Jan 3, 2015
jtimon renamed this:
~~Move CTxOut::IsDust() to policy.o~~
Policy: Move CTxOut::IsDust() to policy.o
on Jan 3, 2015
jtimon commented at 3:44 PM on January 3, 2015: contributor

Sorry for touching it again, moved to 2 commits instead of 3. Now it shares the first commit with #5180.
sipa commented at 3:19 PM on January 4, 2015: member

Untested ACK
laanwj added the label TX fees and policy on Jan 8, 2015
jtimon force-pushed on Jan 12, 2015
jtimon force-pushed on Jan 12, 2015
jtimon force-pushed on Jan 21, 2015
jtimon force-pushed on Jan 21, 2015
jtimon commented at 1:41 PM on January 21, 2015: contributor

I am very sorry for delaying this again, but I'm closing until it's clear how the first step for policy should be. Something I invite reviewers to discuss at #5595 where I've placed some examples.
jtimon closed this on Jan 21, 2015
jtimon reopened this on Jul 12, 2015
jtimon force-pushed on Jul 12, 2015
jtimon commented at 2:23 PM on July 12, 2015: contributor

Reopened. I was waiting for ~~#5595~~ #6068, but no real reason to keep waiting. I'm also moving CFeeRate out of consensus to policy/fees (previously was necessary for primitives/transaction).
jtimon force-pushed on Jul 16, 2015

Policy: Refactor: CTxOut::IsDust(CFeeRate) -> CPolicy::ValidateOutput(CTxOut)

Decouples CTxOut from CFeeRate
Simplifies IsDust() interface encapsulating the access to global minRelayTxFee

c14381c57b

MOVEONLY: Consensus: Policy: Move CFeeRate from amount.o to policy/fees.o e2851a2c8c
jtimon force-pushed on Jul 16, 2015
jtimon commented at 6:36 PM on July 20, 2015: contributor

Needs rebase and I'm thinking about moving the functions to the fee estimator (policy/fees) instead of policy/policy, so closing for now.
jtimon closed this on Jul 20, 2015
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Refactoring TX fees and policy

Linked (view graph)

#5071 Introduce CNodePolicy for putting isolated node policy code and parameters on #5100 MOVEONLY: Split Transaction and Block from core #5652 Policy: Continue policy movements #7820 Consensus: Policy: Move CFeeRate out of consensus module and create CPolicy interface