rpc method listreceivedbyaddress doesn’t take address as a string #14173

issue RHavar openend this issue on September 9, 2018
  1. RHavar commented at 0:52 am on September 9, 2018: contributor
    0bitcoin-cli listreceivedbyaddress 0 true true 2NABbUr9yeRCp1oUCtVmgJF8HGRCo3ifpTT
    

    gives an error: error: Error parsing JSON:2NABbUr9yeRCp1oUCtVmgJF8HGRCo3ifpTT

    as it’s trying to parse the address as json. so you’d have to write:

    0bitcoin-cli listreceivedbyaddress 0 true true '"2NABbUr9yeRCp1oUCtVmgJF8HGRCo3ifpTT"'
    

    which is clumsy and inconsistent with other RPC methods as well as the help file

  2. fanquake added the label RPC/REST/ZMQ on Sep 9, 2018
  3. laanwj commented at 10:12 am on September 15, 2018: member

    can you try the following patch?

     0diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
     1index f4d44fc809bffc4c40fbbb5882bcdb2d4a4be720..438d1cb5cb158e0093671093477bec156248d516 100644
     2--- a/src/rpc/client.cpp
     3+++ b/src/rpc/client.cpp
     4@@ -45,7 +45,6 @@ static const CRPCConvertParam vRPCConvertParams[] =
     5     { "listreceivedbyaddress", 0, "minconf" },
     6     { "listreceivedbyaddress", 1, "include_empty" },
     7     { "listreceivedbyaddress", 2, "include_watchonly" },
     8-    { "listreceivedbyaddress", 3, "address_filter" },
     9     { "listreceivedbylabel", 0, "minconf" },
    10     { "listreceivedbylabel", 1, "include_empty" },
    11     { "listreceivedbylabel", 2, "include_watchonly" },
    

    (the fourth argument to listreceivedbyaddress being called address_filter suggests that it can take other JSON expressions, such as a list, but it’s really only a JSON string, so there’s no reason to treat it differently from other string parameters)

    apparently this was mistakingly introduced in 8ee08120de0b6765d8b9081e06f743e15653f8e4

  4. RHavar commented at 3:35 am on October 4, 2018: contributor
    Looks good. Also I think the same thing is happening with the new getblockstats
  5. MarcoFalke added the label good first issue on Oct 4, 2018
  6. MarcoFalke added the label Up for grabs on Oct 4, 2018
  7. MarcoFalke added the label Hacktoberfest on Oct 4, 2018
  8. MarcoFalke commented at 3:57 am on October 4, 2018: member
    Would be nice to submit the patch with a corresponding test to get this covered?
  9. etscrivner referenced this in commit e57140b5e9 on Oct 6, 2018
  10. etscrivner referenced this in commit ccdcd9eec9 on Oct 6, 2018
  11. etscrivner commented at 8:47 pm on October 6, 2018: contributor

    Can confirm that something similar is happening with getblockstats by making the following changes to the rpc_getblockstats.py tests:

     0diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py
     1index b24bed6ad..0317e63b8 100755
     2--- a/test/functional/rpc_getblockstats.py
     3+++ b/test/functional/rpc_getblockstats.py
     4@@ -51,6 +51,9 @@ class GetblockstatsTest(BitcoinTestFramework):
     5     def get_stats(self):
     6         return [self.nodes[0].getblockstats(hash_or_height=self.start_height + i) for i in range(self.max_stat_pos+1)]
     7
     8+    def get_stats_cli(self):
     9+        return [self.nodes[0].cli.getblockstats(self.start_height + i) for i in range(self.max_stat_pos+1)]
    10+
    11     def generate_test_data(self, filename):
    12         mocktime = time.time()
    13         self.nodes[0].generate(101)
    14@@ -176,5 +179,14 @@ class GetblockstatsTest(BitcoinTestFramework):
    15         assert_raises_rpc_error(-5, 'Block not found', self.nodes[0].getblockstats,
    16                                 hash_or_height='000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f')
    17
    18+        # Test for regressions on [#14173](/bitcoin-bitcoin/14173/)
    19+        cli_stats = self.get_stats_cli()
    20+        assert_equal(set(cli_stats[0].keys()), set(expected_keys))
    21+
    22+        for i in range(self.max_stat_pos+1):
    23+            blockhash = self.expected_stats[i]['blockhash']
    24+            stats_by_hash = self.nodes[0].cli.getblockstats(hash_or_height=blockhash)
    25+            assert_equal(stats_by_hash, self.expected_stats[i])
    26+
    27 if __name__ == '__main__':
    28     GetblockstatsTest().main()
    

    The fix here is less straightforward since the first parameter can be either a number (in which case it needs to be converted using ParseNonRFCJSONValue) or a string (in which case conversion will cause a JSON parsing error). At present conversion is always attempted, causing blockhashes to fail unless explicitly enclosed with double quotes. The crux of the issue being this line here where paths diverge for string and numeric RPC parameters in a way which is not currently reconcilable.

  12. etscrivner commented at 6:59 pm on October 12, 2018: contributor
    I’ve created a simple fix (#14417) for the initial issue described here. From my initial analysis above it seems as though fixing other similar issues may involve rethinking ParseNonRFCJsonValue and its associated code a bit.
  13. etscrivner referenced this in commit f80c9efdee on Oct 14, 2018
  14. etscrivner referenced this in commit d4d70eda33 on Oct 14, 2018
  15. MarcoFalke closed this on Oct 23, 2018

  16. MarcoFalke referenced this in commit 94cf1ec948 on Oct 23, 2018
  17. MarcoFalke referenced this in commit b845f7f13b on Oct 23, 2018
  18. MarcoFalke referenced this in commit 1fcd1553a0 on Oct 28, 2018
  19. jfhk referenced this in commit c95cc5a9e3 on Nov 14, 2018
  20. MarcoFalke referenced this in commit 7e126405f9 on Nov 20, 2018
  21. JeremyRubin referenced this in commit 462deeb201 on Nov 24, 2018
  22. HashUnlimited referenced this in commit 18ad3333b0 on Nov 26, 2018
  23. MarcoFalke referenced this in commit fb9ad043f8 on Nov 28, 2018
  24. MarcoFalke removed the label Hacktoberfest on Sep 30, 2019
  25. MarcoFalke removed the label Up for grabs on Sep 30, 2019
  26. MarcoFalke removed the label good first issue on Sep 30, 2019
  27. Munkybooty referenced this in commit c401cab21e on Jul 21, 2021
  28. PastaPastaPasta referenced this in commit ea26a81cea on Aug 16, 2021
  29. vijaydasmp referenced this in commit 604c23cde1 on Sep 8, 2021
  30. vijaydasmp referenced this in commit 0927f06b1f on Sep 8, 2021
  31. vijaydasmp referenced this in commit d79308d0c9 on Sep 8, 2021
  32. vijaydasmp referenced this in commit 9f091c7042 on Sep 9, 2021
  33. vijaydasmp referenced this in commit 334be22a47 on Sep 9, 2021
  34. vijaydasmp referenced this in commit 87fb3da13e on Sep 9, 2021
  35. DrahtBot locked this on Dec 16, 2021
  36. gades referenced this in commit 75b5e67536 on May 31, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me