Skip to content

Conversation

@TheBlueMatt
Copy link
Contributor

This fixes a potential bug where some NATs may replace the node's
interal IP with its external IP in version messages, causing
incorrect checksums when version messages begin being checksummed
on February 14, 2012.

@TheBlueMatt
Copy link
Contributor Author

0.5.X version: TheBlueMatt@a910838

This fixes a potential bug where some NATs may replace the node's
interal IP with its external IP in version messages, causing
incorrect checksums when version messages begin being checksummed
on February 14, 2012.
@gmaxwell
Copy link
Contributor

This is a good idea regardless of the NAT issue.

There is no reason to leak internal non-routable addresses in version messages, and that part of the patch is clearly safe because using a proxy has the same effect (also because we don't actually do anything with that data).

Using UPNP to get our external IP is also a good idea. The current dependance on centralized address identifying services is a bad one.

@gavinandresen
Copy link
Contributor

ACK

1 similar comment
@sipa
Copy link
Member

sipa commented Feb 11, 2012

ACK

@gmaxwell
Copy link
Contributor

ACK, took me a while to setup upnp to test this.

gmaxwell added a commit that referenced this pull request Feb 11, 2012
Get ext. IP from UPnP, make sure addrMe IsRoutable() in version.
@gmaxwell gmaxwell merged commit 9f3de58 into bitcoin:master Feb 11, 2012
coblee referenced this pull request in litecoin-project/litecoin Jul 17, 2012
Get ext. IP from UPnP, make sure addrMe IsRoutable() in version.
destenson pushed a commit to destenson/bitcoin--bitcoin that referenced this pull request Jun 26, 2016
- Terrible bug which caused governance objects to have their hashes corrupted, causing objects to become invalid
- More proposal -> governance name changes
ptschip added a commit to ptschip/bitcoin that referenced this pull request Dec 19, 2017
* Uncache any valid coins when trimming the orphan cache

* Only remove coins from the coins cache if they failed to enter the mempool

However, do not remove them if the txn was an Orphan.  We need to keep
orphaned coins until and unless those orphans get trimmed from the
orphan cache.

* Update log message when checkinputs fails for a tx

* fMissingInputs must be defined when calling AcceptToMemoryPool

* Add UncacheTx() to Coins.cpp and use it

* UnCache coins when txns are expired from the mempool

* Add more detailed explanation about how vCoinsToUncache works.

It's not entirely intuitive as to why the tracking happens at
this point in AcceptToMemoryPoolWorker().

* Remove unnecessary if(pfMissingInputs)

This clarifies the intent of the code. If we're missing inputs
then this is an orphan and we don't want to uncache the coins.

* Make sure to cache all coins in an orphan

Some orphans may have valid inputs and one or more missing inputs.
Make sure to get the valid ones into cache before returning from
AcceptToMemoryPoolWorker().

* Add tests for uncaching coins

Disable mocktime at end of test

move txvalidationcache_tests.cpp to the original place in the Makefile

Make nLastOrphanCheck and extern and use it

 When we use EraseOrphanByTime() in the test suite we
 must set the nLastOrphanCheck time also otherwise tests that
 run earlier can impact tests that run afterward resulting in
 orphans that do not get erased when they should be.

* Handle pfMissingInputs if passed as a NULL.

* formatting

* change where *pfMissingIputs is set to false
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Feb 28, 2019
bae276a Don't bother checking obfuscation messages (Fuzzbawls)
837f25f [Wallet] Fix segfault with runtime -disablewallet (Fuzzbawls)

Tree-SHA512: a8dd502897cdc82f206a1805f7b1ae51960344c3fa32640c9dc6c271768e7db718bc2f57034e8fabfd1b402e589e531419d1e8af8a25eee71d89a6b79ce3a679
Losangelosgenetics pushed a commit to Losangelosgenetics/bitcoin that referenced this pull request Mar 12, 2020
@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.

4 participants