add DEFAULT_UPNP constant in net #4132

1. 
   Diapolo commented at 7:09 PM on May 5, 2014: none

   • as this is a shared Core/GUI setting, this makes it easier to keep them in sync (also no new includes are needed)
2. add DEFAULT_UPNP constant in net

   - as this is a shared Core/GUI setting, this makes it easier to keep them
     in sync (also no new includes are needed)

   d4e1c61212
3. in src/net.cpp:None in f8074dfde8 outdated

   1740 | @@ -1741,10 +1741,8 @@ void StartNode(boost::thread_group& threadGroup)
   1741 |      else
   1742 |          threadGroup.create_thread(boost::bind(&TraceThread<void (*)()>, "dnsseed", &ThreadDNSAddressSeed));
   1743 |  
   1744 | -#ifdef USE_UPNP
   1745 |      // Map ports with UPnP
   1746 | -    MapPort(GetBoolArg("-upnp", USE_UPNP));
   1747 | -#endif
   1748 | +    MapPort(GetBoolArg("-upnp", DEFAULT_UPNP));
   

   ---

   ---

   

   laanwj commented at 7:13 PM on May 5, 2014:

   This doesn't look correct. You shouldn't call MapPort at all if USE_UPNP is not set.

   ---

   

   Diapolo commented at 6:01 AM on May 6, 2014:

   We have an empty dummy function that is called if USE_UPNP is not set (in net.cpp).

   ---

   

   laanwj commented at 8:35 AM on May 8, 2014:

   OK

4. in src/net.h:None in f8074dfde8 outdated

     37 | @@ -38,6 +38,12 @@ namespace boost {
     38 |  
     39 |  /** The maximum number of entries in an 'inv' protocol message */
     40 |  static const unsigned int MAX_INV_SZ = 50000;
     41 | +/** -upnp default */
     42 | +#ifdef USE_UPNP
     43 | +static const bool DEFAULT_UPNP = true;
   

   ---

   ---

   

   laanwj commented at 7:15 PM on May 5, 2014:

   There are three settings for USE_PNP:

   • #undef USE_UPNP: no libupnpc compiled in at all
   • #define USE_UPNP 0: libupnp compiled in, but defaults to off
   • #define USE_UPNP 1: libupnp compiled in, and defaults to on

   ---

   

   Diapolo commented at 6:03 AM on May 6, 2014:

   If USE_UPNP is defined, we default to true, all other cases (even if no libupnpc was compiled in) should be false, IMO this should be working?

   ---

   

   laanwj commented at 8:35 AM on May 8, 2014:

   No - USE_UPNP can be defined and be either 0 or 1, depending on whether it should be enabled by default or not. So this should be:

   static const bool DEFAULT_UPNP = USE_UPNP;
   

   ---

   

   Diapolo commented at 1:22 PM on May 20, 2014:

   Yeah, will change this then...

   Edit: Changed!

5. 
   BitcoinPullTester commented at 3:30 PM on May 20, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/d4e1c61212c3f28f80c7184aca81f5d118fad460 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

6. 
   laanwj commented at 9:15 AM on May 21, 2014: member

   ACK

7. laanwj added this to the milestone 0.10.0 on May 21, 2014
8. 
   Diapolo commented at 9:26 AM on May 29, 2014: none

   @laanwj Any reason not to pull this (checking my open commits for progress currently)?

9. laanwj merged this on May 29, 2014
10. laanwj closed this on May 29, 2014

    ---

11. laanwj referenced this in commit 67f43a99ae on May 29, 2014
12. Diapolo deleted the branch on May 29, 2014
13. DrahtBot locked this on Sep 8, 2021

Contributors

DiapololaanwjBitcoinPullTester

Milestone
0.10.0