Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
jtimon wants to merge 2 commits into from
Closed

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

jtimon wants to merge 2 commits into from

Conversation

@jtimon
Copy link
Contributor

@jtimon jtimon commented Oct 21, 2014

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

Diapolo
Diapolo reviewed Oct 21, 2014
View reviewed changes
src/chainparams.h Outdated
Copy link

@Diapolo Diapolo Oct 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Should be below checkpoints.

@jtimon jtimon force-pushed the dust branch 3 times, most recently from 14aab3e to ec74278 Compare October 28, 2014 13:31
@jtimon
Copy link
Contributor Author

jtimon commented Oct 28, 2014

Rebased to make it more readable.

@sipa
Copy link
Member

sipa commented Oct 29, 2014

If you're moving it, can you move it to main instead? It really doesn't belong in core..

@jtimon jtimon force-pushed the dust branch 2 times, most recently from 9a8a9be to 2d6dc1c Compare October 30, 2014 12:28
@jtimon
Copy link
Contributor Author

jtimon commented Oct 30, 2014

Moved it to policy.o, where I think it belongs.

@sipa
Copy link
Member

sipa commented Nov 4, 2014

utACK

@jtimon
Copy link
Contributor Author

jtimon commented Nov 5, 2014

We have to take into account that this will conflict with #5071.

@jtimon
Copy link
Contributor Author

jtimon commented Dec 27, 2014

Needed rebase (was including core/transaction instead of privitives/transaction).

@petertodd
Copy link
Contributor

petertodd commented Dec 28, 2014

utACK

@jtimon
Copy link
Contributor Author

jtimon commented Dec 29, 2014

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 jtimon force-pushed the dust branch 2 times, most recently from d127235 to 1652df6 Compare December 29, 2014 16:11
This was referenced Dec 30, 2014
Closed
Closed
@petertodd
Copy link
Contributor

petertodd commented Jan 3, 2015

utACK

@jtimon jtimon changed the title Turn CTxOut::IsDust() into an independent function Move CTxOut::IsDust() to policy.o Jan 3, 2015
@jtimon jtimon changed the title Move CTxOut::IsDust() to policy.o Policy: Move CTxOut::IsDust() to policy.o Jan 3, 2015
@jtimon
Copy link
Contributor Author

jtimon commented Jan 3, 2015

Sorry for touching it again, moved to 2 commits instead of 3. Now it shares the first commit with #5180.

@jtimon jtimon mentioned this pull request Jan 3, 2015
Closed
@sipa
Copy link
Member

sipa commented Jan 4, 2015

Untested ACK

@jtimon jtimon mentioned this pull request Jan 8, 2015
Closed
@jtimon jtimon force-pushed the dust branch 2 times, most recently from 3cbfc5a to eff5330 Compare January 12, 2015 23:13
@jtimon jtimon mentioned this pull request Jan 13, 2015
Closed
@jtimon jtimon force-pushed the dust branch 2 times, most recently from fc5f6d5 to e6d753e Compare January 21, 2015 13:36
@jtimon
Copy link
Contributor Author

jtimon commented Jan 21, 2015

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 jtimon closed this Jan 21, 2015
@jtimon jtimon mentioned this pull request Jun 6, 2015
Closed
@jtimon jtimon reopened this Jul 12, 2015
@jtimon
Copy link
Contributor Author

jtimon commented Jul 12, 2015

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 added 2 commits July 16, 2015 20:28
@jtimon
Policy: Refactor: CTxOut::IsDust(CFeeRate) -> CPolicy::ValidateOutput…
c14381c 
…(CTxOut)

Decouples CTxOut from CFeeRate
Simplifies IsDust() interface encapsulating the access to global minRelayTxFee
@jtimon
MOVEONLY: Consensus: Policy: Move CFeeRate from amount.o to policy/fe…
e2851a2 
…es.o
@jtimon
Copy link
Contributor Author

jtimon commented Jul 20, 2015

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 jtimon closed this Jul 20, 2015
@jtimon jtimon mentioned this pull request Feb 26, 2016
Closed
@jtimon jtimon mentioned this pull request Mar 16, 2016
Closed
@jtimon jtimon mentioned this pull request Apr 5, 2016
Closed
@jtimon jtimon mentioned this pull request Feb 11, 2017
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Refactoring TX fees and policy

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jtimon @sipa @petertodd @Diapolo @laanwj