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.

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

Add "mempool" P2P command #1470

Closed
wants to merge 2 commits into from
Closed

Add "mempool" P2P command #1470

wants to merge 2 commits into from

Conversation

jgarzik
Copy link
Contributor

@jgarzik jgarzik commented Jun 15, 2012

This adds the "mempool" P2P command, which returns the first 50,000 entries in the TX memory pool (CInv vectors are capped at 50,000 already). SPV clients and other nodes may use this command following a block sync-up, to ensure they have the latest snapshot of the collective network.

@@ -2869,6 +2884,14 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
}


else if (strCommand == "mempool")
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do an if (fClient) return;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely not. This is part of SPV mode (although there are other uses as well). SPV clients sync up blocks, and then look in the mempool for any transactions they may have missed.

This is also a client-initiated command, so it is the client that is proactively selecting to receive this data.

Copy link
Contributor

Choose a reason for hiding this comment

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

if (fClient) would mean that if we are a SPV node, we should not respond to mempool requests, as other SPV clients should not be asking peers that can entirely not check transactions for transactions (really to make it more explicit and keep SPV implementors from doing stupid things)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thin clients can and will know at a higher level whether or not they should issue 'mempool' command in the first place. Behavior for thin and fat clients and thin and fat servers is adequate without an explicit fClient check.

Copy link
Contributor

Choose a reason for hiding this comment

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

From IRC:
If we are redefining NODE_NETWORK (==!fClient) to be a classic-style archival node not necessarily a full node (for pruning), then Im fine with defining a SUPPORT_SPV_CLIENTS to be has full blocks and supports mempool/filters/etc then Im fine with that. As long as we dont have partial support for the final SUPPORT_SPV_CLIENTS-type flag in one version and finalize it in the next.

@gavinandresen
Copy link
Contributor

How should this be tested?

@jgarzik
Copy link
Contributor Author

jgarzik commented Jun 15, 2012

@gavinandresen just a simple "it works" test using ArtForz' half-a-node satisfies me...

@luke-jr
Copy link
Member

luke-jr commented Jun 19, 2012

Shouldn't this do something to ensure the transactions inv'd can be fetched? bitcoind won't send just any transaction right now, only ones it thinks it's flood-relaying.

@mikehearn
Copy link
Contributor

Yeah, Luke is right. The patch needs to update the getdata command as well.

After I'm finished with LevelDB I can throw together some bitcoinj support for using this command and make sure it works.

@jgarzik
Copy link
Contributor Author

jgarzik commented Jul 31, 2012

Foo Dog

@jgarzik jgarzik closed this Jul 31, 2012
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 22, 2019
in GBT, match fees and sigops with the correct tx, despite CTOR ordering.  mining_ctor.py test written by @schancel and ported by @awemany
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request May 6, 2020
1ef737b [GUI] coin control: add icon and tooltip for delegated UTXOs (random-zebra)

Pull request description:

  Implemented on top of:
  - [x] bitcoin#1469

  (only last commit is new)

  **Problem**: As outlined in the description of bitcoin#1391, delegated outputs are not clearly distinguishable in the coin control dialog.

  **Proposed Solution**: Add a "cold staking icon" (purple snow flake) at the end of delegated utxo rows. Hovering on this icon shows a tooltip with the staker address which has received the delegation.

  ![ccontrol1](https://user-images.githubusercontent.com/18186894/77853074-650b2c00-71e2-11ea-8908-926b39f9e230.png)

  <br>

  ![ccontrol2](https://user-images.githubusercontent.com/18186894/77853078-69374980-71e2-11ea-839a-53a2b0a83d20.png)

ACKs for top commit:
  Fuzzbawls:
    ACK 1ef737b
  furszy:
    utACK 1ef737b .

Tree-SHA512: 4da7c478ef8b2463bc80e002e25f6da4b2e18e47afbb024fa344945dbbe5dfc3b0fb75c6e821d67d19b373cf123aad1c827dfcf37b412d0fda5d8cd279aa5fff
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants