Skip to content

fix hang / deadlock with wallet filter enabled and other patches#277

Merged
ca333 merged 20 commits intoGLEECBTC:devfrom
DeckerSU:patch-lock
Jan 27, 2020
Merged

fix hang / deadlock with wallet filter enabled and other patches#277
ca333 merged 20 commits intoGLEECBTC:devfrom
DeckerSU:patch-lock

Conversation

@DeckerSU
Copy link
Copy Markdown

This PR should fix daemon hang / deadlock with wallet filter enabled, also it have some fixes / patches from various upstreams (BTC, ZEC):

  • code refactoring: move komodo internal structures update from wait to shutdown loop into separate thread, which seemed more logic. wait for shutdown loop almost restored to it's original state from ZEC.
  • miner: remove komodo_longestchain call from miner internal loop, we defenitely don't want to iterate all connected nodes stats on each miner nOnce iteration.
  • wallet filter: fully remove old blackjok3r's wallet filter code.
  • wallet filter: new wallet filter implementation, key difference - now if enabled it allows txes not only from notary address and addresses listed in -whitelistaddress, but also from all addresses belongs to a walelt. in other words - if any addresses passed via -whitelistaddress, wf is enabled and allows txes only from addresses that belongs to wallet and white-listed addresses. be aware to use wf on z-tx (private tx) enabled chains, bcz it possible can reject tx from z->t. it designed only for ac_public chains only and KMD itself.
  • code refactoring: remove creatsig error message and keystore.%p error message on stderr.
  • rpc: work-around an upstream libevent bug
  • GH actions: fixed windows and linux build (removed php7.4-fpm package)
  • code refactoring: remove dead code in init
  • CVE-2018–20586 fix

move komodo_passport_iteration and komodo_cbopretupdate from
WaitForShutdown loop to separate thread ThreadUpdateKomodoInternals()
for KMD it should be 10, according to prev implementation in
WaitForShutdown, for assetchains it should be ASSETCHAINS_BLOCKTIME/5
should be +1 bcz it was loop for(i=0; i<=ASSETCHAINS_BLOCKTIME/5;i++)
in original loop.
we don't need iterate all nodes stats inside a miner to know
max of (nStartingHeight, nSyncHeight, nCommonHeight) to get
longestchain. it's wasting CPU resources. also we don't need
additional locks directly in miner.
If any vout of tx is belongs to wallet (IsMine(tx) == true) and tx is not from us, mean,
if every vin not belongs to our wallet (IsFromMe(tx) == false), then tx need to be checked
through wallet filter. If tx haven't any vin from trusted / whitelisted address it shouldn't
be added into wallet.

Using GetTransaction instead of myGetTransaction to get prevTx and
determine vins addresses. This will allow walletfilter code to work
without txindex=1 on any (!) nodes, not only notaries. If even one
-whitelistaddress arg set wallet filter is enabled and should allow
incoming txes from any addresses belongs to wallet (even watch-only)
and addresses listed in -whitelistaddress args. If -whitelistaddress
arg is not set wallet filter is disabled.
A rare race condition may trigger while awaiting the body of a message, see
upsteam commit libevent/libevent@5ff8eb2 for details.

This may fix some reported rpc hangs/crashes.

Issue: bitcoin/bitcoin#11593

Commits:
- bitcoin/bitcoin@6b58360
- bitcoin/bitcoin@97932cd
@ca333 ca333 self-requested a review January 27, 2020 15:15
@ca333 ca333 changed the base branch from beta to dev January 27, 2020 15:34
@ca333 ca333 requested review from smk762 and tonymorony January 27, 2020 17:52
@ca333 ca333 merged commit dbdb0a9 into GLEECBTC:dev Jan 27, 2020
who-biz pushed a commit to who-biz/komodo that referenced this pull request Jul 29, 2024
Update deprecation height and version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants