Skip to content
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

[rpc] mempoolinfo should take ::minRelayTxFee into account #11475

Merged
merged 1 commit into from Dec 23, 2017
Merged

[rpc] mempoolinfo should take ::minRelayTxFee into account #11475

merged 1 commit into from Dec 23, 2017

Conversation

mess110
Copy link
Contributor

@mess110 mess110 commented Oct 10, 2017 •
edited

Fixes #6941 following #11410 (comment) 's suggestion

This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

By modifying

bitcoin/src/txmempool.cpp

Line 984 in 5a9da37

return CFeeRate(llround(rollingMinimumFeeRate));
the syncing mempools becomes problematic as per #11410 (comment)

Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong? travis sometimes fails unexpectedly

@mess110 mess110 force-pushed the include_minRelayTxFee_in_getmempoolinfo_rpc branch from ab0c8cb to 9717aab Compare October 10, 2017 16:59
@mess110 mess110 force-pushed the include_minRelayTxFee_in_getmempoolinfo_rpc branch from 9717aab to 149dffd Compare October 11, 2017 20:39
ryanofsky
ryanofsky reviewed Oct 12, 2017
View reviewed changes
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

utACK 149dffd. This is fine, but I definitely prefer #11410 because it cleaned up a bunch of code and got rid of weird edge cases.

@mess110
Copy link
Contributor Author

mess110 commented Oct 12, 2017 •
edited

@ryanofsky I sort of prefer that too, but I couldn't figure out (yet) why I had those random mempool sync problems. Figured I would contribute some more to gain more knowledge, and figure that out at a later point in time.

Thanks for the review

Changing https://github.com/bitcoin/bitcoin/blob/master/src/txmempool.cpp#L986 is what causes the sporadic mempool sync problems

@TheBlueMatt
Copy link
Contributor

utACK 149dffd

@jeffrade jeffrade mentioned this pull request Dec 19, 2017
Closed
@jeffrade
Copy link
Contributor

@mess110 Looks like #6941 is only open for updating RPC help getmempoolinfo. Possible to make that update here in this PR? If not, I can make a new PR once yours is merged.

@laanwj
Copy link
Member

laanwj commented Dec 23, 2017 •
edited

utACK 149dffd

I definitely prefer #11410

Doesn't look like that one is still active, and it's been a long time, so I'm just going to merge this one.

Possible to make that update here in this PR? If not, I can make a new PR once yours is merged.

Yes, better to do that in a new change.

@laanwj laanwj merged commit 149dffd into bitcoin:master Dec 23, 2017
laanwj added a commit that referenced this pull request Dec 23, 2017
@laanwj
Merge #11475: [rpc] mempoolinfo should take ::minRelayTxFee into account
9bad8d6 
149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes #6941 following #11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per #11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
@jeffrade jeffrade mentioned this pull request Dec 24, 2017
Merged
@mess110 mess110 deleted the include_minRelayTxFee_in_getmempoolinfo_rpc branch December 27, 2017 10:15
laanwj added a commit that referenced this pull request Jan 4, 2018
@laanwj
Merge #12001: [RPC] Adding ::minRelayTxFee amount to getmempoolinfo a…
a9a49e6 
…nd updating help

aad3090 [rpc] Adding ::minRelayTxFee amount to getmempoolinfo and updating mempoolminfee help description (Jeff Rade)

Pull request description:

  These are RPC document changes from #11475 which is now merged.  Took into consideration comments from #11475 and #6941 for this PR.

  Biggest change here is when calling `getmempoolinfo`, will now show the `minrelaytxfee` in the JSON reponse (see below):

  ```
  $ bitcoin-cli getmempoolinfo
  {
    "size": 50,
    "bytes": 13102,
    "usage": 70480,
    "maxmempool": 300000000,
    "mempoolminfee": 0.00001000,
    "minrelaytxfee": 0.00001000
  }
  ```

  Fixes #8953

Tree-SHA512: 5ca583961365ee1cfe6e0d19afb0b41d542e179efee3b3c5f3fcf7d3ebca9cc3eedfd1434a0da40c5eed84fba98b35646fda201e6e61c689b58bee9cbea44b9e
@meshcollider meshcollider mentioned this pull request Jan 17, 2018
Closed
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 17, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
a3d6179 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 22, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
3b2be13 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 22, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
d7e6d28 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
1a18074 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
1bb00b2 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
33e8986 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 31, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11475: [rpc] mempoolinfo should take ::minRelayTxFee in…
b8c5d5d 
…to account

149dffd [rpc] mempoolinfo should take ::minRelayTxFee into account (Cristian Mircea Messel)

Pull request description:

  Fixes bitcoin#6941 following bitcoin#11410 (comment) 's suggestion

  This takes care of the mentioned ticket without changing the behavior of https://github.com/bitcoin/bitcoin/pull/11410/files#diff-24efdb00bfbe56b140fb006b562cc70bL629

  By modifying https://github.com/bitcoin/bitcoin/blob/5a9da37fb3f4b53f556e1d46509b94dc3c661d75/src/txmempool.cpp#L984 the syncing mempools becomes problematic as per bitcoin#11410 (comment)

  ~~Same code causes different tests to fail: https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 https://travis-ci.org/bitcoin/bitcoin/jobs/286128241 . I can't reproduce the problems locally, am I doing something wrong?~~ travis sometimes fails unexpectedly

Tree-SHA512: fd81628da6a3eff51bd09e5342d781bac0710f79d6b330b1df3662756ecaceb2e1682bf9768b5f8edbcba6479a3223dfa6604d37c9e9d37d00d077172da4f6ea
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12001: [RPC] Adding ::minRelayTxFee amount to getmempoo…
a0abbd4 
…linfo and updating help

aad3090 [rpc] Adding ::minRelayTxFee amount to getmempoolinfo and updating mempoolminfee help description (Jeff Rade)

Pull request description:

  These are RPC document changes from bitcoin#11475 which is now merged.  Took into consideration comments from bitcoin#11475 and bitcoin#6941 for this PR.

  Biggest change here is when calling `getmempoolinfo`, will now show the `minrelaytxfee` in the JSON reponse (see below):

  ```
  $ bitcoin-cli getmempoolinfo
  {
    "size": 50,
    "bytes": 13102,
    "usage": 70480,
    "maxmempool": 300000000,
    "mempoolminfee": 0.00001000,
    "minrelaytxfee": 0.00001000
  }
  ```

  Fixes bitcoin#8953

Tree-SHA512: 5ca583961365ee1cfe6e0d19afb0b41d542e179efee3b3c5f3fcf7d3ebca9cc3eedfd1434a0da40c5eed84fba98b35646fda201e6e61c689b58bee9cbea44b9e
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12001: [RPC] Adding ::minRelayTxFee amount to getmempoo…
a2c8d92 
…linfo and updating help

aad3090 [rpc] Adding ::minRelayTxFee amount to getmempoolinfo and updating mempoolminfee help description (Jeff Rade)

Pull request description:

  These are RPC document changes from bitcoin#11475 which is now merged.  Took into consideration comments from bitcoin#11475 and bitcoin#6941 for this PR.

  Biggest change here is when calling `getmempoolinfo`, will now show the `minrelaytxfee` in the JSON reponse (see below):

  ```
  $ bitcoin-cli getmempoolinfo
  {
    "size": 50,
    "bytes": 13102,
    "usage": 70480,
    "maxmempool": 300000000,
    "mempoolminfee": 0.00001000,
    "minrelaytxfee": 0.00001000
  }
  ```

  Fixes bitcoin#8953

Tree-SHA512: 5ca583961365ee1cfe6e0d19afb0b41d542e179efee3b3c5f3fcf7d3ebca9cc3eedfd1434a0da40c5eed84fba98b35646fda201e6e61c689b58bee9cbea44b9e
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12001: [RPC] Adding ::minRelayTxFee amount to getmempoo…
4e6cd82 
…linfo and updating help

aad3090 [rpc] Adding ::minRelayTxFee amount to getmempoolinfo and updating mempoolminfee help description (Jeff Rade)

Pull request description:

  These are RPC document changes from bitcoin#11475 which is now merged.  Took into consideration comments from bitcoin#11475 and bitcoin#6941 for this PR.

  Biggest change here is when calling `getmempoolinfo`, will now show the `minrelaytxfee` in the JSON reponse (see below):

  ```
  $ bitcoin-cli getmempoolinfo
  {
    "size": 50,
    "bytes": 13102,
    "usage": 70480,
    "maxmempool": 300000000,
    "mempoolminfee": 0.00001000,
    "minrelaytxfee": 0.00001000
  }
  ```

  Fixes bitcoin#8953

Tree-SHA512: 5ca583961365ee1cfe6e0d19afb0b41d542e179efee3b3c5f3fcf7d3ebca9cc3eedfd1434a0da40c5eed84fba98b35646fda201e6e61c689b58bee9cbea44b9e
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12001: [RPC] Adding ::minRelayTxFee amount to getmempoo…
67cf959 
…linfo and updating help

aad3090 [rpc] Adding ::minRelayTxFee amount to getmempoolinfo and updating mempoolminfee help description (Jeff Rade)

Pull request description:

  These are RPC document changes from bitcoin#11475 which is now merged.  Took into consideration comments from bitcoin#11475 and bitcoin#6941 for this PR.

  Biggest change here is when calling `getmempoolinfo`, will now show the `minrelaytxfee` in the JSON reponse (see below):

  ```
  $ bitcoin-cli getmempoolinfo
  {
    "size": 50,
    "bytes": 13102,
    "usage": 70480,
    "maxmempool": 300000000,
    "mempoolminfee": 0.00001000,
    "minrelaytxfee": 0.00001000
  }
  ```

  Fixes bitcoin#8953

Tree-SHA512: 5ca583961365ee1cfe6e0d19afb0b41d542e179efee3b3c5f3fcf7d3ebca9cc3eedfd1434a0da40c5eed84fba98b35646fda201e6e61c689b58bee9cbea44b9e
@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

@ryanofsky ryanofsky ryanofsky left review comments

Assignees
No one assigned
Labels
RPC/REST/ZMQ
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mempoolminfee returns incorrect data before the dynamic mempool limit has changed
6 participants
@mess110 @TheBlueMatt @jeffrade @laanwj @ryanofsky @fanquake