rpc: Return block hash & height in getbalances, gettransaction and getwalletinfo #26094

pull aureleoules wants to merge 1 commits into bitcoin:master from aureleoules:return-blockhash-with-wallet-calls changing 9 files +70 −6
  1. aureleoules commented at 5:21 PM on September 14, 2022: member

    Reopens #18570 and closes #18567. I have rebased the original PR. Not sure why the original got closed as it was about to get merged.

  2. aureleoules force-pushed on Sep 14, 2022
  3. DrahtBot added the label RPC/REST/ZMQ on Sep 14, 2022
  4. DrahtBot commented at 10:31 PM on September 14, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK achow101
    Concept ACK jarolrod
    Stale ACK willcl-ark

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
    • #25634 (wallet, tests: Expand and test when the blank wallet flag should be un/set by achow101)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  5. luke-jr commented at 1:33 AM on September 20, 2022: member

    As I said previously,

    I'm not sure this is the right approach, interface-wise, especially for calls like gettransaction, where the info gets embedded in the JSON transaction itself.

    Maybe it'd be better to just guarantee atomicity for batch requests?

  6. in test/functional/wallet_balance.py:327 in 7bf1c59b9c outdated 
     310 | +        prev_hash = self.nodes[0].getbestblockhash()
 311 | +        prev_height = self.nodes[0].getblock(prev_hash)['height']
 312 | +        self.generatetoaddress(self.nodes[0], 5, self.nodes[0].get_deterministic_priv_key().address)
 313 | +        lastblock = self.nodes[0].getbalances()['lastprocessedblock']
 314 | +        assert_is_hash_string(lastblock['hash'])
 315 | +        assert_equal((prev_hash == lastblock['hash']), False)
    jarolrod commented at 6:49 AM on September 23, 2022:

    there's also assert_not_equal that could be used

    aureleoules commented at 4:45 PM on October 3, 2022:

    assert_not_equal does not exist :neutral_face:

    jarolrod commented at 4:51 PM on October 3, 2022:

    right, sorry, mind was mixed up while reviewing: https://github.com/bitcoin/bitcoin/pull/23127

  7. in test/functional/wallet_balance.py:337 in 7bf1c59b9c outdated 
     318 | +        self.log.info("Test getwalletinfo returns expected lastprocessedblock json object")
 319 | +        walletinfo = self.nodes[0].getwalletinfo()
 320 | +        assert_greater_than(walletinfo['lastprocessedblock']['height'], 0)
 321 | +        assert_is_hash_string(walletinfo['lastprocessedblock']['hash'])
 322 | +
 323 | +        self.log.info("Test gettransaction returns expected lastprocessedblock json object")
    jarolrod commented at 6:52 AM on September 23, 2022:

    should also have a test on the height to ensure that this is in fact the "expected" lastprocessedblock json object?

    aureleoules commented at 4:59 PM on October 3, 2022:

    Done.

  8. jarolrod commented at 6:54 AM on September 23, 2022: member

    Concept ACK

    This does what it says it does. I think it's fine to add these fields, I can imagine it would be useful. Tested and sanity checked the code, and ran tests locally.

  9. DrahtBot added the label Needs rebase on Oct 3, 2022
  10. aureleoules force-pushed on Oct 3, 2022
  11. aureleoules force-pushed on Oct 3, 2022
  12. aureleoules force-pushed on Oct 3, 2022
  13. DrahtBot removed the label Needs rebase on Oct 3, 2022
  14. willcl-ark approved
  15. willcl-ark commented at 8:02 PM on November 21, 2022: member

    ACK https://github.com/bitcoin/bitcoin/commit/37a05b4f6bd768873cdfaaf0b417ef7d29c60f97

    Returns the blockheight and hash the aforementioned RPCs were run against. Feels like a win for users of the RPCs.

  16. achow101 commented at 10:56 PM on February 3, 2023: member

    ACK 37a05b4f6bd768873cdfaaf0b417ef7d29c60f97

    Maybe it'd be better to just guarantee atomicity for batch requests?

    I don't really see how that would be possible to implement in a way that wouldn't be detrimental or fragile. It seems like it would have to be implemented with an explicit list of RPCs that must be done atomically in order to avoid any RPC just locking things up. Such a list could fall out of date as new RPCs are added and people forget, so it would be kind of fragile.

  17. achow101 commented at 1:00 PM on April 26, 2023: member

    There's a test failure in wallet_orphanedreward.py.

    256/263 - wallet_orphanedreward.py failed, Duration: 17 s

stdout:
2023-04-26T12:57:28.311000Z TestFramework (INFO): PRNG seed is: 5570008732445289618
2023-04-26T12:57:28.313000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20230426_085557/wallet_orphanedreward_19
2023-04-26T12:57:40.283000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/andy/bitcoin/bitcoin/master/test/functional/test_framework/test_framework.py", line 132, in main
    self.run_test()
  File "/home/andy/bitcoin/bitcoin/master/gcc_build/enable-debug/../../test/functional/wallet_orphanedreward.py", line 68, in run_test
    assert_equal(self.nodes[1].getbalances(), pre_reorg_conf_bals)
  File "/home/andy/bitcoin/bitcoin/master/test/functional/test_framework/util.py", line 56, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not({'mine': {'trusted': Decimal('35.00000000'), 'untrusted_pending': Decimal('0E-8'), 'immature': Decimal('0E-8')}, 'lastprocessedblock': {'hash': '20e9d23d7183d38eb6789704c3e658d6915178fd95d4479edfd5c3f2df023793', 'height': 305}} == {'mine': {'trusted': Decimal('35.00000000'), 'untrusted_pending': Decimal('0E-8'), 'immature': Decimal('0E-8')}, 'lastprocessedblock': {'hash': '5503776f6d7a7dc6f769176922582a6333a0d6821ec0c2dc30f9114f45fa2351', 'height': 302}})
2023-04-26T12:57:40.334000Z TestFramework (INFO): Stopping nodes
2023-04-26T12:57:40.436000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20230426_085557/wallet_orphanedreward_19
2023-04-26T12:57:40.436000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20230426_085557/wallet_orphanedreward_19/test_framework.log
2023-04-26T12:57:40.436000Z TestFramework (ERROR): 
2023-04-26T12:57:40.436000Z TestFramework (ERROR): Hint: Call /home/andy/bitcoin/bitcoin/master/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20230426_085557/wallet_orphanedreward_19' to consolidate all logs
2023-04-26T12:57:40.436000Z TestFramework (ERROR): 
2023-04-26T12:57:40.436000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-04-26T12:57:40.436000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2023-04-26T12:57:40.436000Z TestFramework (ERROR): 


stderr:
  18. rpc: return block hash & height in getbalances, gettransaction & getwalletinfo JSONs
    Co-authored-by: Aurèle Oulès <aurele@oules.com>
    710b83938a
  19. aureleoules force-pushed on Apr 26, 2023
  20. DrahtBot added the label CI failed on Apr 26, 2023
  21. DrahtBot removed the label CI failed on Apr 26, 2023
  22. achow101 commented at 3:42 PM on May 2, 2023: member

    ACK 710b83938ab5bbc4bd324d8b2e69461a2a1d2eec

  23. DrahtBot requested review from willcl-ark on May 2, 2023
  24. achow101 merged this on May 2, 2023
  25. achow101 closed this on May 2, 2023
  26. sidhujag referenced this in commit ab8a2a0751 on May 4, 2023
  27. luke-jr referenced this in commit 63f63d9011 on Aug 16, 2023
  28. bitcoin locked this on May 1, 2024
Contributors
aureleoulesDrahtBotluke-jrjarolrodwillcl-arkachow101


willcl-ark

Labels
RPC/REST/ZMQ
Linked (view graph)
#18567 Return block hash with wallet calls#18570 rpc: return block hash & height in getbalances, gettransaction & getwalletinfo JSONs
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: 2026-04-13 15:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me