Add missing cs_main lock in rest/getutxos #11766

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2017/11/rest_getutxo_lockfix changing 1 files +13 −4

jonasschnelli commented at 8:34 AM on November 25, 2017: contributor

The rest getutxos call introduced in #5420 does serval access of chainActive without holding cs_main. Discovered during code review of #11765.
Add missing cs_main lock in rest/getutxos 1410dcdc8f
jonasschnelli added the label RPC/REST/ZMQ on Nov 25, 2017
promag commented at 11:10 AM on November 25, 2017: member

Echoing @luke-jr concern in another PR: in line 490 cs_main is locked and IMO you should read chainActive there, so consistency is guaranteed in the call result.
TheBlueMatt commented at 2:57 PM on November 25, 2017: member
Is this a dup of #11596?

On November 25, 2017 12:34:50 AM PST, Jonas Schnelli notifications@github.com wrote:
The rest getutxos call introduced in https://github.com/bitcoin/bitcoin/pull/5420 does serval access of chainActive without holding cs_main. Discovered during code review of #11765. You can view, comment on, or merge this pull request online at:

#11766

-- Commit Summary --

Add missing cs_main lock in rest/getutxos

-- File Changes --

M src/rest.cpp (17)

-- Patch Links --

https://github.com/bitcoin/bitcoin/pull/11766.patch https://github.com/bitcoin/bitcoin/pull/11766.diff

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/11766
jonasschnelli commented at 7:45 PM on November 25, 2017: contributor

Oh. Yes. Closing this in favour of #11596
jonasschnelli closed this on Nov 25, 2017
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5420 [REST] getutxos REST command (based on Bip64)#11765 [REST] added blockhash api, tests and documentation