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

Feature request: sendtoname #5

Closed
cassiniNMC opened this issue May 25, 2015 · 14 comments · Fixed by #6

Comments

@cassiniNMC
Copy link

commented May 25, 2015

Currently this functionality can already be achieved by a combination of two RPCs (name_show and sendtoaddress). However, the sendtoname feature could become an important "unique selling proposition" in the future. Therefore sendtoname should be an RPC in its own right, I think.

domob1812 added a commit to domob1812/namecore that referenced this issue May 26, 2015
This fixes namecoin#5.
@domob1812

This comment has been minimized.

Copy link

commented May 26, 2015

Please test: #6

domob1812 added a commit to domob1812/namecore that referenced this issue May 28, 2015
This fixes namecoin#5.
domob1812 added a commit to domob1812/namecore that referenced this issue May 28, 2015
This fixes namecoin#5.
@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 1, 2015

I'm not really convinced that this is something we should encourage. As I understand it (and please correct me if I'm wrong), using sendtoname will produce transactions that are easily linkable, both to each other and to the name. While some users may intend for this information to be public (not really sure what use case it would be, maybe some kind of publicly auditable payment), the vast majority of users either will want it to be private or don't really care (but if they don't care, that usually means they just don't realize what linkability implies, and it should be private by default).

It seems to me that a better architecture would be to have the name's value include a payment address (which could be an ECDH address) and have the client send to that address. If an ECDH address is used, then the payments will not be as easily linkable.

I realize that ECDH addresses are not well-standardized and are an area of active development. But I do think that we should future-proof ourselves for widespread use of ECDH addresses (or other reusable addresses like zerocoin addresses), and sendtoname is incapable of this as far as I can tell.

@domob1812

This comment has been minimized.

Copy link

commented Jun 1, 2015

It is not a big feature and probably not something that a lot of people will use (as it is RPC-only), but it is still nice to have. It is a feature we can trivially have in Namecoin, and which improves usability a lot compared to other coins - so much so that it would, basically, remove the need for both Onename and Netki. That they got significant VC funding seems to imply that the feature is not so unimportant.

Of course, it has privacy implications - but so do addresses associated to id/. Are you also in favour of removing them from the id/ spec and NameID? I think it is nice to have for simplicity, and people who may use it must at least be familiar with the RPC interface (and know about the command in the first place) - so they can be expected to know what they are doing.

Are you definitely opposing merging this pull request, or is it ok for you nevertheless?

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 1, 2015

Well, as I said, an id/ name can list an address that is safe to reuse (i.e. an ECDH address like DarkWallet and sx use). That's not possible when using the script that corresponds to the owner of the name. So I certainly support having id/ include payment addresses -- I'd just hope that users use ECDH addresses with them (or zerocoin addresses if those become a thing later). Is there a disadvantage to using a payment address from the name's value rather than the name's owner script? I realize that it would require parsing the JSON of the name's value, but this doesn't seem to outweight the privacy advantage of such an approach, in my opinion. (And NMControl could easily do this; @phelixbtc and I were discussing doing so a while back.)

Unfortunately there are users who do surprisingly stupid things with RPC and/or command line tools, particularly when some third party writes a "n00b friendly howto guide" that doesn't disclose the privacy risks. (I've been asked in the past to repair various bad things that novices did due to following bad howtos that they didn't understand, and I never cease being amazed at what damage can happen in such cases.)

I would like to get some further discussion before we decide on whether to support this API.

domob1812 added a commit to domob1812/namecore that referenced this issue Jun 1, 2015
This fixes namecoin#5.
@phelixbtc

This comment has been minimized.

Copy link

commented Jun 2, 2015

There is no difference in privacy between sendtoname and having a normal address in the value. For now we simply don't have stealth addresses (of course that would be a feature great to have).

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 2, 2015

The difference is that using the name value will future proof us for stealth addresses. Stealth addresses should work fine on the Namecoin network right now, just no clients utilize them. Is there a disadvantage to using the name value?

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 3, 2015

To elaborate now that I'm back at my laptop (was on my phone earlier) -- I'm against introducing features that will need to be deprecated in the near future. While stealth addresses are not used by Namecoin users at the moment, they will be in the near future presumably. Using the name value means that we will not have to adjust anything when stealth addresses (or zerocoin addresses) become supported. It is impossible to support stealth addresses or zerocoin addresses by using the name's owner script (which is what sendtoname does). There is no reason that I'm aware of to not use the name value.

@phelixbtc

This comment has been minimized.

Copy link

commented Jun 3, 2015

This is not about introducing a new feature. It has been present for a long time in the old client. We can not simply change it's behavior. We should introduce a new rpc command for using an address from the value, maybe specifically for stealth addresses.

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 8, 2015

@phelixbtc It is true that this functionality has been in NamecoinQ for a while. However, since we're pressuring end users to go to some effort to migrate to Namecoin Core anyway, it seems that the switch to Namecoin Core is a convenient time to remove that functionality. If we don't remove it now, then I would support (at the least) marking it as dangerous in the documentation, and removing it in the future (let's say 6 months after NMControl merges support for using the JSON field for this functionality). The fact that a little-known feature existed for some time, is not (by itself) a solid reason to keep it indefinitely IMO.

domob1812 added a commit to domob1812/namecore that referenced this issue Jun 10, 2015
This fixes namecoin#5.
@cassiniNMC

This comment has been minimized.

Copy link
Author

commented Jun 12, 2015

then I would support (at the least) marking it as dangerous

If sendtoname is dangerous then Namecoin is dangerous. Or put it the other way around, if we remove sendtoname with the intention of improving privacy then this would lead to a false sense of safety. There is still sendtoaddress which is much more dangerous as it pretends to be anonymous.

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 12, 2015

@cassiniNMC That isn't really the case. sendtoaddress can (in principle) use ECDH addresses (e.g. stealth addresses), which are safe to reuse, and they can also (right now) use unique addresses if the user supplies them. sendtoname, by its nature, cannot use ECDH addresses, nor can it use unique addresses. Therefore sendtoname is substantially more dangerous than sendtoaddress.

@phelixbtc

This comment has been minimized.

Copy link

commented Jun 15, 2015

We can't randomly cripple functionality just with a vague hint that there might be something better coming up.

@JeremyRand

This comment has been minimized.

Copy link
Member

commented Jun 15, 2015

@phelixbtc Fine. I will propose deprecating/removing sendtoname from Namecoin Core approximately 6-12 months after both value-based sending and reusable addresses have been implemented in NMControl and wallets. I will not NACK sendtoname until that has happened, since it is already in the NamecoinQ API and there is not a great alternative at the moment. Is that okay?

@phelixbtc

This comment has been minimized.

Copy link

commented Jun 15, 2015

@JeremyRand Yep, sounds like a good plan.

domob1812 added a commit to domob1812/namecore that referenced this issue Jun 18, 2015
domob1812 added a commit to domob1812/namecore that referenced this issue Jun 24, 2015
domob1812 added a commit to domob1812/namecore that referenced this issue Jun 24, 2015
@domob1812 domob1812 closed this in #6 Jun 25, 2015
domob1812 pushed a commit that referenced this issue Dec 10, 2016
003a4ac Merge #5: fix typo
5254f14 [trivial] Fix typo
e7c0aab Merge #4: Fix some comments
d07cead Fix some comments

git-subtree-dir: src/crypto/ctaes
git-subtree-split: 003a4acfc273932ab8c2e276cde3b4f3541012dd
domob1812 pushed a commit that referenced this issue Aug 8, 2017
c521b3ac6 Merge #11: fixup define checks. Cleans up some oopses from #5.
8b1cd3753 fixup define checks. Cleans up some oopses from #5.
6b1508d6d Merge #6: Fixes typo
fceb80542 Merge #10: Clean up compile-time warnings (gcc 7.1)
0ec2a343f Clean up compile-time warnings (gcc 7.1)
d4c268a35 Merge #5: Move helper functions out of sse4.2 object
8d4eb0847 Add HasAcceleratedCRC32C to port_win.h
77cfbfd25 crc32: move helper functions out of port_posix_sse.cc
4c1e9e016 silence compiler warnings about uninitialized variables
495316485 Merge #2: Prefer std::atomic over MemoryBarrier
2953978ef Fixes typo
f134284a1 Merge #1: Merge upstream LevelDB 1.20
ba8a445fd Prefer std::atomic over MemoryBarrier

git-subtree-dir: src/leveldb
git-subtree-split: c521b3ac654cfbe009c575eacf7e5a6e189bb5bb
domob1812 pushed a commit that referenced this issue Aug 8, 2017
b13a68e Squashed 'src/leveldb/' changes from 196962ff0..c521b3ac6 (Pieter Wuille)

Pull request description:

  Includes:
  * bitcoin-core/leveldb#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille)
  * bitcoin-core/leveldb#5: Move helper functions out of sse4.2 object (Cory Fields)
  * bitcoin-core/leveldb#6: Fixes typo (Dimitris Tsapakidis)
  * bitcoin-core/leveldb#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo)
  * bitcoin-core/leveldb#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields)

Tree-SHA512: 2b88a99a86ed8c74c860de13a123ea7f5424d35d314be564820cf83aaae8308383403f7cd56f17c241cfee4885699796141fed666559c21044eaabaeea073315
domob1812 pushed a commit that referenced this issue Feb 24, 2018
51d3ab34ba Merge #10: Add pushKV(key, boolean) function (replaces #5)
129bad96d5 [tests] test pushKV for boolean values
b3c44c947f Pushing boolean value to univalue correctly

git-subtree-dir: src/univalue
git-subtree-split: 51d3ab34ba2857f0d03dc07250cb4a2b5e712e67
domob1812 pushed a commit that referenced this issue Aug 6, 2018
6f53edb Acquire cs_main before ATMP call in block_assemble bench (James O'Beirne)

Pull request description:

  Calling `bench_bitcoin` currently fails due to calling ATMP without acquiring cs_main first in the recently added block_assemble bench (bitcoin/bitcoin#13219).

  ```
  $ cat <(uname -a) <(gcc --version)

  Linux james 4.4.0-119-generic #143+jamesob SMP Mon Apr 16 21:47:24 EDT 2018 x86_64 x86_64 x86_64 GNU/Linux
  gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609

  $ ./src/bench/bench_bitcoin

  WARNING: This is a debug build - may result in slower benchmarks.
  # Benchmark, evals, iterations, total, min, max, median
  Assertion failed: lock cs_main not held in validation.cpp:566; locks held:
  [1]    19323 abort (core dumped)  ./src/bench/bench_bitcoin
  ```

  ```
  (gdb) bt
  #0  0x00007fbdc9cf5428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
  #1  0x00007fbdc9cf702a in __GI_abort () at abort.c:89
  #2  0x0000555a19580dc5 in AssertLockHeldInternal (pszName=pszName@entry=0x555a19834549 "cs_main",
      pszFile=pszFile@entry=0x555a1988a001 "validation.cpp", nLine=nLine@entry=566, cs=cs@entry=0x555a19ba55c0 <cs_main>) at sync.cpp:157
  #3  0x0000555a194b395f in AcceptToMemoryPoolWorker (chainparams=..., pool=..., state=...,
      ptx=std::shared_ptr (count 1, weak 0) 0x555a1bb819b0, pfMissingInputs=pfMissingInputs@entry=0x0, nAcceptTime=1532964079,
      plTxnReplaced=0x0, bypass_limits=false, nAbsurdFee=@0x7ffcbc1719d8: 0, coins_to_uncache=std::vector of length 0, capacity 0,
      test_accept=false) at validation.cpp:566
  #4  0x0000555a194ba661 in AcceptToMemoryPoolWithTime (chainparams=..., pool=..., state=...,
      tx=std::shared_ptr (count 1, weak 0) 0x555a1bb819b0, pfMissingInputs=pfMissingInputs@entry=0x0, nAcceptTime=<optimized out>,
      plTxnReplaced=0x0, bypass_limits=false, nAbsurdFee=0, test_accept=false) at validation.cpp:998
  #5  0x0000555a194ba7ce in AcceptToMemoryPool (pool=..., state=..., tx=std::shared_ptr (count 1, weak 0) 0x555a1bb819b0,
      pfMissingInputs=pfMissingInputs@entry=0x0, plTxnReplaced=plTxnReplaced@entry=0x0, bypass_limits=bypass_limits@entry=false, nAbsurdFee=0,
      test_accept=false) at validation.cpp:1014
  #6  0x0000555a19363fbe in AssembleBlock (state=...) at bench/block_assemble.cpp:102
  #7  0x0000555a193654d3 in std::_Function_handler<void (benchmark::State&), void (*)(benchmark::State&)>::_M_invoke(std::_Any_data const&, benchmark::State&) (__functor=..., __args#0=...) at /usr/include/c++/5/functional:1871
  #8  0x0000555a193501d7 in std::function<void (benchmark::State&)>::operator()(benchmark::State&) const (this=this@entry=0x555a1ba2cda0,
      __args#0=...) at /usr/include/c++/5/functional:2267
  #9  0x0000555a1934ec4c in benchmark::BenchRunner::RunAll (printer=..., num_evals=5, scaling=<optimized out>, filter=..., is_list_only=false)
      at bench/bench.cpp:121
  #10 0x0000555a1934ade9 in main (argc=<optimized out>, argv=<optimized out>) at bench/bench_bitcoin.cpp:92
  ```

Tree-SHA512: fdd7b28ff123ccea7a4f334d53f735d0c0f94aa9cc52520c2dd34dca45d78c691af64efcd32366fc472fedffbd79591d2be2bb3bfc4a5186e8712b6b452d64e3
domob1812 pushed a commit that referenced this issue Jun 19, 2019
… non-deterministic

c061be1 tests: Mark unit test blockfilter_index_initial_sync as non-deterministic (practicalswift)

Pull request description:

  Mark unit test `blockfilter_index_tests/blockfilter_index_initial_sync` as non-deterministic.

  Before this PR:

  ```
  $ contrib/devtools/test_deterministic_coverage.sh 500
  [2019-06-04 09:58:57] Measuring coverage, run #1 of 500
  [2019-06-04 10:00:33] Measuring coverage, run #2 of 500
  [2019-06-04 10:02:19] Measuring coverage, run #3 of 500

  The line coverage is non-deterministic between runs. Exiting.

  The test suite must be deterministic in the sense that the set of lines executed at least
  once must be identical between runs. This is a necessary condition for meaningful
  coverage measuring.

  --- gcovr.run-1.txt     2019-06-04 10:00:33.389059973 +0000
  +++ gcovr.run-3.txt     2019-06-04 10:03:45.619491207 +0000
  @@ -72,7 +72,7 @@
   hash.h                                        54      33    61%   71,74-77,82,85-89,111,113,128,147-148,175,178-181
   httprpc.cpp                                  120       3     2%   31,34-35,38-40,46,49,52,54,56,58,70,73-74,76,78-79,81,83-84,89,91,94-95,97,99-101,103,106-107,111-112,117-119,121-122,125,128,130,132,134-136,138-139,142,145,148,151-153,156-160,163-166,171,173-175,180-182,185,187,189-190,192,195,198-199,201,203-204,212,215,217,219-222,224,227-228,230,232,237,239-240,243-245,247-251,254,256,259,261-264,266-267 [* 205-206,208-209]
   httpserver.cpp                               312       6     1%   46,49-50,53,55,80-81,90,92-93,96-98,101,104,106-109,111-112,114,118,120-122,126,128-129,153,155,157-158,164,166-178,180,182,184-188,192,194-196,198-199,201-202,204-205,207-208,213,216-221,225,228-232,236-239,243-244,247-254,256-258,264-267,270-271,274,279,281-282,286,288-290,292-293,297,299-300,303-307,309-310,312-317,322-328,330,332,335,339,341-342,346,352-353,355,358,360,364,368-369,375,378,381-384,388-391,393-394,398-400,402,404-406,409,411-412,414,416,426,428-431,433-434,438,440-441,443,445-446,449,451-455,457-459,463-464,466-469,471-473,475-477,479,482,484,487,490-493,496-497,499-500,502,504,506,508-509,511,513-514,517,519,521-522,527,529-533,535,538,540-543,550-555,558,560-562,570,572-574,577-582,585-590,594-597,600,602-604,606-609,611,614,616,619,621,625-626,628-629,631-632,634-635,640,642-643,646,648-651,653,655-656
  -index/base.cpp                               149      94    63%   20,22-25,28,66,98,102-103,117-118,140-141,145-146,155,163,175,177-178,181-182,184-185,200-201,203,212,214-215,219-221,228-229,234,236,240,243-244,247-249,258-260,262,270,292-294,308-309 [* 263]
  +index/base.cpp                               149      97    65%   20,22-25,28,66,98,102-103,117-118,140-141,145-146,155,163,175,177-178,181-182,184-185,200-201,203,212,214-215,219-221,228-229,234,236,240,243-244,247-249,258-260,262,270,308-309 [* 263]
   index/base.h                                   3       2    66%   77
   index/blockfilterindex.cpp                   199     134    67%   70,79,81,84-88,91,122,139,142,179-181,184-185,188-189,193-194,201-202,207,233,258,262-263,265-266,268,271-272,274,277,279,284,286,288-289,294,301-302,304,322,329,332-333,350,371,373,438,440-441,444,446,449,455-456,459,461,464,466 [* 162-163]
   index/blockfilterindex.h                       4       4   100%
  @@ -358,7 +358,7 @@
   util/validation.cpp                            5       1    20%   12,15-17
   validation.cpp                              2167     808    37%   291,293,297-300,302,330,332,340,348,355-357,359,362,364-365,368,371,380,382-383,385-386,388-389,396,398-402,406-413,415,417,419,422-425,439-440,442-443,446,449,455-458,461-464,467,469-470,472,474,476,492,494-495,502-503,505-507,511-513,515,517,523,526,528,533,535,540,542-544,550,552-556,558-560,564,574,578-583,586,590-591,594-596,601-602,607-608,611-612,616-617,619-621,635-636,638,640,647-648,651,657-658,660-662,665-667,673,675,677-678,682-683,690,693,700-701,703-705,709-710,713-714,716,719-720,724-727,733-735,737-739,741-743,747-748,751-752,754,757-764,771,773-774,776-779,785-788,793-794,796-800,815-816,818-822,825,827,830,835,838-839,841-843,846-848,850,853,859,864-867,875,877-879,884-885,887-891,895,899-900,904-906,908-909,911,930-931,933,936,942,944-950,952,959,962,965-968,972,978,982-984,990-991,994-996,999,1003-1004,1011,1013,1015-1019,1022-1023,1026-1032,1056,1065,1079,1091,1108,1112,1114-1118,1125,1127-1130,1133-1135,1138-1139,1147,1149,1151-1152,1155,1197,1199-1201,1206-1209,1211-1212,1226,1230,1232-1234,1236,1238-1241,1245-1246,1256,1258,1260-1262,1264-1266,1268,1278-1280,1282-1283,1286,1289,1291-1292,1294-1302,1305-1311,1319-1323,1330,1332-1333,1336-1339,1379,1383-1384,1395,1401,1405-1407,1411-1414,1423-1428,1438-1440,1451,1455,1458,1471,1480,1497,1503,1519,1525,1527-1530,1532-1533,1536,1538-1539,1549,1551,1553,1555,1559-1562,1571,1573,1578,1580,1582-1584,1588-1589,1594-1597,1601-1606,1613-1616,1619-1623,1630,1632,1635,1637,1639-1640,1642-1646,1658,1660,1675,1688,1711,1713-1715,1742,1755,1760,1765,1769,1811,1815,1817,1841-1845,1855,1942,1946-1947,1956,1984-1986,1991-1992,1994,1996-1999,2005-2007,2010-2012,2022-2023,2028-2031,2038-2039,2042,2044,2049,2058-2061,2064,2114-2115,2117-2118,2120-2124,2152-2153,2156,2159-2163,2165-2169,2171-2172,2176-2178,2187-2188,2191-2194,2199,2207-2211,2215-2220,2224,2227-2230,2235,2237-2238,2261-2263,2265,2274,2278,2286,2301,2303-2304,2306-2309,2311,2313-2318,2320,2322,2325,2327-2328,2330,2332-2334,2338,2340,2343-2344,2407-2410,2430,2445-2447,2507-2509,2511-2514,2518,2520-2521,2523-2524,2561,2564,2590,2592-2593,2595-2598,2603,2620,2626,2658,2719,2724,2773,2776-2777,2779,2781,2783,2785-2788,2791,2793-2795,2799,2801-2802,2805,2807-2809,2813,2816,2818-2821,2825-2826,2832-2834,2841-2845,2848,2854,2858-2859,2861,2865-2868,2872-2875,2880,2884-2885,2890-2891,2894-2895,2897,2900-2906,2908,2910,2912,2918-2922,2924,2928-2929,2940,3002-3005,3009-3010,3026-3028,3036-3037,3039-3040,3045,3053,3056,3077,3080,3090,3112,3118,3129,3133,3135-3136,3141-3142,3150,3190-3193,3259,3268,3273,3277,3282-3285,3303,3314,3321-3324,3338-3341,3345-3346,3348-3350,3360,3372,3392,3397,3403,3406,3408,3435-3441,3443,3468-3469,3485,3487-3488,3492-3493,3534-3536,3542,3547-3549,3552,3565-3566,3601-3602,3610,3628,3630,3632,3645,3647,3649-3651,3653,3657,3659,3661-3669,3675-3680,3686-3687,3691,3693-3697,3702,3704,3706-3708,3711-3718,3720,3724,3726-3729,3748,3750-3752,3754,3758-3759,3763,3765,3767,3772,3774,3777-3778,3780-3781,3783,3787-3788,3790,3792-3794,3798-3800,3823,3825,3828,3830,3832,3836-3838,3841-3843,3845,3848,3850,3854-3856,3858-3859,3861-3862,3864-3867,3870-3873,3875-3876,3879,3882-3883,3886-3893,3899,3901,3905-3909,3911-3915,3922-3924,3926-3928,3931,3933-3934,3940-3942,3945-3947,3952,3954-3955,3957,3960-3961,3964,3966,3968-3972,3975,3977,3980,3982,3985,3987-3988,3992-3996,3998-4006,4008-4009,4011-4012,4014,4016,4019,4021-4022,4024-4026,4028-4032,4037-4041,4043-4045,4047,4050,4053-4054,4057,4060-4064,4066-4067,4069-4075,4079-4080,4086,4089-4091,4094-4097,4101,4106,4108,4110,4112-4114,4116-4117,4119,4121,4123-4124,4126,4128-4130,4132-4134,4138-4142,4144-4147,4154,4158-4163,4166-4169,4172-4173,4177,4179-4180,4183,4185,4187-4189,4191-4193,4195,4197-4201,4207-4208,4212,4220-4223,4230,4232-4233,4237,4240,4243,4247,4249,4251,4253-4255,4265-4266,4277,4279,4282,4285-4287,4292-4293,4296,4298,4302,4305-4306,4310-4311,4315-4318,4360,4363-4367,4370,4377,4397,4412,4415-4416,4418,4421-4422,4424,4426-4429,4433-4437,4439-4441,4448-4452,4454-4456,4458,4460,4462-4467,4471-4475,4477,4480-4481,4486-4488,4493,4496-4503,4505,4507-4511,4513-4514,4517-4519,4529-4531,4546,4600,4638-4639,4647,4653,4662-4664,4696,4703-4704,4718,4720,4723,4725,4727,4730,4732-4733,4736,4738-4739,4742,4744-4745,4750,4752-4757,4761-4765,4769-4770,4774-4776,4779-4781,4783-4785,4787-4790,4793-4794,4800-4801,4803,4807,4809-4810,4812-4813,4815-4816,4823,4827,4829,4831-4832,4834-4835,4838-4840,4842,4845,4848-4849,4853,4855-4856,4858-4863,4866-4872,4877,4891,4907 [* 1085-1086,1140-1141,1513-1514,2201-2202,2428,3569-3570,4400-4401,4442,4453,4504,4522-4523,4526-4527,4818-4819,4873-4874]
   validation.h                                  19       5    26%   338,350-352,356-363,366,484
  -validationinterface.cpp                       81      50    61%   78-82,85-86,112-113,116,119-120,123-124,126-128,130,133-136,151-153,163-165,169-171
  +validationinterface.cpp                       83      60    72%   78-82,85-86,112-113,116,133-136,151-153,163-165,169-171
   validationinterface.h                          9       4    44%   94,105,112,118,135
   versionbits.cpp                               92      27    29%   33,35-36,38-39,48-50,52-54,56-57,61-62,67-71,73,75-76,80,82-83,91,98,100,102-103,105,109-110,113-118,121-122,124,127,129-130,134,137,141,149,151,153-155,159,177,179,184,194,196,199,201,204,206 [* 26]
   versionbits.h                                  1       1   100%
  @@ -400,5 +400,5 @@
   zmq/zmqpublishnotifier.h                       5       0     0%   12,31,37,43,49
   zmq/zmqrpc.cpp                                23       3    13%   16,18,20,23,33-35,37,40-47,51,62,64-65
   ------------------------------------------------------------------------------
  -TOTAL                                      52472    7784    14%
  +TOTAL                                      52474    7797    14%
   ------------------------------------------------------------------------------
  $
  ```

  After this PR:

  ```
  $ contrib/devtools/test_deterministic_coverage.sh 500
  [2019-06-03 14:45:25] Measuring coverage, run #1 of 500
  [2019-06-03 14:48:15] Measuring coverage, run #2 of 500
  [2019-06-03 14:50:49] Measuring coverage, run #3 of 500
  [2019-06-03 14:52:20] Measuring coverage, run #4 of 500
  [2019-06-03 14:53:49] Measuring coverage, run #5 of 500
  …
  [2019-06-04 09:04:58] Measuring coverage, run #496 of 500
  [2019-06-04 09:07:42] Measuring coverage, run #497 of 500
  [2019-06-04 09:10:32] Measuring coverage, run #498 of 500
  [2019-06-04 09:13:26] Measuring coverage, run #499 of 500
  [2019-06-04 09:16:32] Measuring coverage, run #500 of 500

  Coverage test passed: Deterministic coverage across 500 runs.
  $
  ```

ACKs for commit c061be:

Tree-SHA512: 00cd55b4371290d8587ab667c64249bc31d26cc9dc3dd519677eb91ddb9dbc5333dfbdef5e90c7a0d74eecd24757113e7ec3eda836859ddc033b1de715df81b6
domob1812 pushed a commit that referenced this issue Jun 24, 2019
…erministic

f899580 tests: Make coins_tests/updatecoins_simulation_test deterministic (practicalswift)

Pull request description:

  Make `coins_tests/updatecoins_simulation_test` deterministic.

  Before:

  ```
  $ contrib/devtools/test_deterministic_coverage.sh 1000
  [2019-06-15 05:36:20] Measuring coverage, run #1 of 1000
  [2019-06-15 05:38:05] Measuring coverage, run #2 of 1000
  [2019-06-15 05:39:49] Measuring coverage, run #3 of 1000
  [2019-06-15 05:41:38] Measuring coverage, run #4 of 1000
  [2019-06-15 05:43:16] Measuring coverage, run #5 of 1000
  ...
  [2019-06-16 18:25:23] Measuring coverage, run #880 of 1000
  [2019-06-16 18:27:12] Measuring coverage, run #881 of 1000
  [2019-06-16 18:29:33] Measuring coverage, run #882 of 1000
  [2019-06-16 18:33:00] Measuring coverage, run #883 of 1000
  [2019-06-16 18:35:32] Measuring coverage, run #884 of 1000

  The line coverage is non-deterministic between runs. Exiting.

  The test suite must be deterministic in the sense that the set of lines executed at least
  once must be identical between runs. This is a necessary condition for meaningful
  coverage measuring.

  --- gcovr.run-1.txt     2019-06-15 05:38:05.282359029 +0200
  +++ gcovr.run-884.txt   2019-06-16 18:37:23.518298374 +0200
  @@ -269,7 +269,7 @@
   test/bloom_tests.cpp                         320     320   100%
   test/bswap_tests.cpp                          13      13   100%
   test/checkqueue_tests.cpp                    223     222    99%   169
  -test/coins_tests.cpp                         478     472    98%   52,68,344-345,511,524
  +test/coins_tests.cpp                         478     474    99%   52,68,511,524
   test/compilerbug_tests.cpp                    18      18   100%
   test/compress_tests.cpp                       27      27   100%
   test/crypto_tests.cpp                        268     268   100%
  @@ -401,5 +401,5 @@
   zmq/zmqpublishnotifier.h                       5       0     0%   12,31,37,43,49
   zmq/zmqrpc.cpp                                23       3    13%   16,18,20,23,33-35,37,40-47,51,62,64-65
   ------------------------------------------------------------------------------
  -TOTAL                                      53323   28305    53%
  +TOTAL                                      53323   28307    53%
   ------------------------------------------------------------------------------
  ```

  After:

  ```
  $ contrib/devtools/test_deterministic_coverage.sh 1000
  [2019-06-15 05:36:20] Measuring coverage, run #1 of 1000
  [2019-06-15 05:38:05] Measuring coverage, run #2 of 1000
  [2019-06-15 05:39:49] Measuring coverage, run #3 of 1000
  [2019-06-15 05:41:38] Measuring coverage, run #4 of 1000
  [2019-06-15 05:43:16] Measuring coverage, run #5 of 1000
  ...
  $
  ```

ACKs for commit f89958:
  MarcoFalke:
    ACK f899580 (checked that the randomness state of g_insecure_rand_ctx is the same after three test runs)

Tree-SHA512: 796d362b050c5750e351de1126b62f0f2c8e2d712cf01b6e1a3e2cc6ef92fa68439a32fc24c76d34bce4d553aee4ae4ea88a036c56eb9e25979649a19c59c3e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.