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

Large importdescriptors call slow, possible to reduce number of watched addresses? #715

Closed
casey opened this issue Aug 12, 2022 · 20 comments
Labels
bug Something isn't working module-blockchain
Milestone

Comments

@casey
Copy link

casey commented Aug 12, 2022

BDK is issuing a large importdescriptors call, which is turning out to be very slow. See this issue for some of the gruesome details.

I think the call is large because BDK is asking Bitcoin Core to watch 100 addresses. Is there a way to reduce this number? I don't see it in the RpcBlockchain options.

@casey
Copy link
Author

casey commented Aug 12, 2022

Another option would be to have an option to indicate that you trust the bitcoin core node, and send it an xpubkey-based descriptor. Although this is based off the assumption I'm making that it's the number of addresses that is the problem, and not another issue with importdescriptors.

@rajarshimaitra
Copy link
Contributor

Thanks @casey for reporting this.. This is porobably one more reason to move from "raw" descriptors.. I think core now supports Xpub based descriptors too.. Might be a good time to shift to it..

@casey
Copy link
Author

casey commented Aug 16, 2022

xpub-based descriptors would be great! In the mean time, is there a away to have bdk watch fewer addresses, e.g. watch 10 instead of 100? This is making our tests really slow, so a bandaid would be great.

@rajarshimaitra
Copy link
Contributor

Yes there is.. Set this value in the RpcSyncParam to number of scripts you wanna start with.. The default is 100..

pub start_script_count: usize,

@casey
Copy link
Author

casey commented Aug 17, 2022

Ah, awesome! Thank you!

@terror
Copy link
Contributor

terror commented Aug 19, 2022

Yes there is.. Set this value in the RpcSyncParam to number of scripts you wanna start with.. The default is 100..

pub start_script_count: usize,

Tried reducing the start_script_count to 10 and the importdescriptors call is identical in size to a run with start_script_count set to 100.

Here is the call I am referring to:

[2022-08-19T18:50:51Z DEBUG bitcoincore_rpc] JSON-RPC request: importdescriptors [[{"desc":"raw(00145846369f3d6ba366d6f5a903fb5cf4dca3763c0e)#k9wh6v62","timestamp":0},{"desc":"raw(001420800aabf13f3a4c4ce3ce4c66cecf1d17f21a6e)#6m0hlfh4","timestamp":0},{"desc":"raw(0014c6bf9715e06d73ebf9b3b02d5cc48d24d8bbabc1)#wyavh36r","timestamp":0},{"desc":"raw(00141ba7807b3f46af113beaea5c698428ce7138cd8a)#jctdsups","timestamp":0},{"desc":"raw(00140c1bd27f10fff01b36ddf3c1febaa1acff19b080)#9s6nc3pk","timestamp":0},{"desc":"raw(00141226e31987e4bc2e63c0ee12908f675e40464b20)#9pp7qm39","timestamp":0},{"desc":"raw(0014f73f149f7503960a5e849c6ee7a8a8c336f631cb)#qtkxv9fc","timestamp":0},{"desc":"raw(0014c8ccb4d81ffc769fc5fdd8d7eed69b0e0cae5749)#hn39qayv","timestamp":0},{"desc":"raw(001498565aead2d67a22a6021d55210f2a917fc22169)#6ar3vwsx","timestamp":0},{"desc":"raw(001403013248ac0cd9eabe176cad162cda2a19f771e1)#4m47mukd","timestamp":0},{"desc":"raw(00147de17826fab4e7572755ad8ddbe40ce61f63a699)#03v5x8vf","timestamp":0},{"desc":"raw(00146402f6560278b5c87cf1173f17fd971e3f0e41a1)#0pjd3l95","timestamp":0},{"desc":"raw(0014fc7707f8913a5ed05e9c95f79993fa63fc95fb91)#fdxpnn5d","timestamp":0},{"desc":"raw(00149118bb0f57eea8ba2a8b63c3dd4886ec39b6f559)#v3e0s6wa","timestamp":0},{"desc":"raw(001424f59b558b0094690d208845d2d4aec7eab05917)#qp3f02gu","timestamp":0},{"desc":"raw(001407cfbc97a867f3adc6e74a86a12ab50a6c96dfdc)#nacutv73","timestamp":0},{"desc":"raw(0014ad0e3b494957a6556d48d201dcd08a56a1337e71)#lxs5utkw","timestamp":0},{"desc":"raw(0014a44766df4820e741cf3e69de06604377803b1d72)#xlduxz49","timestamp":0},{"desc":"raw(0014ab3ce65379c43bd52a31245b66482ccad72c56e6)#7m30zf3n","timestamp":0},{"desc":"raw(0014014c4f73107497cbb8a28e95d8558726bcf353b0)#5hn4u62g","timestamp":0},{"desc":"raw(0014e20f4b7479cd94d63dad0af7d422ab318502edc9)#ankgt3u9","timestamp":0},{"desc":"raw(0014a4ad33e67d990ce5dbeda8fc033338e878175bff)#kz76eymj","timestamp":0},{"desc":"raw(00148c59654a57e921c61b36ec55b9f4973809ba3aa7)#4a0gg950","timestamp":0},{"desc":"raw(00141622125f724ae21931c8ad3fcf6d2251a5ff3101)#pa9v83g6","timestamp":0},{"desc":"raw(001492888404b62de9dce757beda671f56622979b277)#0w3ev6uv","timestamp":0},{"desc":"raw(0014e55f7a7a04ccbb3997ac43f9565f7000cc601021)#axc47qx8","timestamp":0},{"desc":"raw(0014f12cb2e9b420c40ff5e0815b1fdc491f30696e82)#pkp43cmf","timestamp":0},{"desc":"raw(0014306057cd7518cd37cec725cbb9057c169913dc0e)#0mv0nk0l","timestamp":0},{"desc":"raw(0014bbbfe144b731a96530b0ce2644482e2e439842bf)#42a244xg","timestamp":0},{"desc":"raw(0014d142df70072bb78d6422e1f5bcd0144a514d1a30)#7d8xpujn","timestamp":0},{"desc":"raw(00141fdf3bb05c1b6d780e9054c89bd619609bf1f82d)#y0x67vfq","timestamp":0},{"desc":"raw(00148939d2d80b6591267c59af6b1614ef28e8505c0d)#w453gc92","timestamp":0},{"desc":"raw(001429b662406f03efe9c20d9227c463aa0c37613768)#mmdtm3vj","timestamp":0},{"desc":"raw(0014339611d35e9e2789fd1c2c4c59744cd528c30448)#5cm5cu7x","timestamp":0},{"desc":"raw(00148db3e38ada7f7da7b853a85564063a34fdbb7bde)#6zvgvvrg","timestamp":0},{"desc":"raw(001493a50be6af43b13ce318c49a5d4088a446348d86)#fxtearpa","timestamp":0},{"desc":"raw(0014c325b3bceb1a06526ed375d6af7e21a3b18d3c4f)#x8kut84a","timestamp":0},{"desc":"raw(0014a86d53c183affc9f41119f9a8a12f1c23ea3f22a)#p307pvnt","timestamp":0},{"desc":"raw(0014c780b4dd93a2bbdbcda22e1b7e26db56ca4538a0)#juna8rk7","timestamp":0},{"desc":"raw(0014e4ed854a807959a7ee787e0f21fd749b23cf7971)#4es3xghl","timestamp":0},{"desc":"raw(0014b75df751f7407cb7adbd281d9f34a34dd5f26b3a)#8hm2usj7","timestamp":0},{"desc":"raw(0014d5ebb3be03f2ad8810fd4e246a60c1a437f27962)#rymmus3n","timestamp":0},{"desc":"raw(001411bbabfb5e10d17e6d29fe6870ac48b9ca560234)#6k8wxlzf","timestamp":0},{"desc":"raw(0014cd701716b718e1cbd474f2f7685c4c7939b18359)#v9rhxfjx","timestamp":0},{"desc":"raw(0014ebf9b6888c54d99b5df846749e397dee0c18f54d)#c79pex2s","timestamp":0},{"desc":"raw(0014d7bea15cd50f69ea154fc906004d1b22eda94eec)#0nj2uyj4","timestamp":0},{"desc":"raw(001489102bee375f6579cbc781267dfe499d0015e85d)#hputch5y","timestamp":0},{"desc":"raw(0014b79ce1bf47da6786ac1e4abbb53a19a691e2d9bf)#x30zcf0g","timestamp":0},{"desc":"raw(00144747c2e3c4a4298919490b62c483c5ae044e54d9)#wme4nj20","timestamp":0},{"desc":"raw(001465bdadb594af71cc17cbab558f9795d12c7cab7f)#nkz9msgr","timestamp":0},{"desc":"raw(001405aa550fd71c6c78144464106e0e2c6698c1da63)#8462trry","timestamp":0},{"desc":"raw(00145d67a29230c89cc8312afbe06eedb4252d043ef4)#yqevjyff","timestamp":0},{"desc":"raw(00140c7eb4ec81793be62515d8683c8ddb6b16151e8e)#pj934vfx","timestamp":0},{"desc":"raw(0014d7f23a11d8539844fc8478e82f19e91f5ccae342)#8w3r73pj","timestamp":0},{"desc":"raw(0014360c671c07af4527a56d83b06648dfab0975a13f)#j6darl3v","timestamp":0},{"desc":"raw(00145d61e5900db40a49346d809e8f4335d68705fff5)#wl06dsvk","timestamp":0},{"desc":"raw(00144b2a163a76f392d975efef958444f6aaac9ddec8)#3rt3rl27","timestamp":0},{"desc":"raw(00147beeca8b2390e7c708d3ba83e05d7e34fc055a8e)#z2uxn97h","timestamp":0},{"desc":"raw(001408a53ef1e2a9ddf2f7b7a4eb574d2b390a2b1c99)#qrjrpl6u","timestamp":0},{"desc":"raw(0014d55cdd2b295c2e3387838fd8b025923edc88ee63)#crf74usx","timestamp":0},{"desc":"raw(0014a38c98d4ba62bb9df9db62a1a8bb58eb2a07a07b)#e2pmfkqm","timestamp":0},{"desc":"raw(0014dd90129a2ed473d5c96f2eb58dca81c01369542e)#y6hsstz4","timestamp":0},{"desc":"raw(0014af7e5643830997210c65f4370eaf5a7dc764d45e)#m7r3gq88","timestamp":0},{"desc":"raw(00149cdf1765b03e1cf5145945394aeddc1a0e6693ac)#mhxdece7","timestamp":0},{"desc":"raw(00148d2e1bac1c70c98e60c34a0e7cea3bca4653f304)#qwpq5fur","timestamp":0},{"desc":"raw(001459755bf4b210e00fa403a0c556e11b842310abde)#4pyt2tu8","timestamp":0},{"desc":"raw(0014b449a14ad2951df4723a5db67fa3ed6da564c09b)#m0v3vj8d","timestamp":0},{"desc":"raw(0014317ddfe4d602826759215e1abea701b9903e486f)#4k8w7762","timestamp":0},{"desc":"raw(00146cc862530d795fe077586bdbb33f511ac7cb01ff)#8eprpvnv","timestamp":0},{"desc":"raw(0014ae91241bb67aba596d1ca6cf6e715b73668625be)#rnhyfshy","timestamp":0},{"desc":"raw(00146c9fa5cc60fc595d839521f43f17a4c6360e2de2)#434dv46f","timestamp":0},{"desc":"raw(0014b089ceabb72af3a98ef4a05f6c456e9f24c671b2)#d4lcthch","timestamp":0},{"desc":"raw(0014771ad9bdde222dd69b4941c648409fbe346aa313)#e4fs2a7y","timestamp":0},{"desc":"raw(0014a8498b2097f007f2ab78c6921041b6d8b8f330de)#dd7pyhz6","timestamp":0},{"desc":"raw(00146faf2f0528e266cb1d06b356cc031d455bd3dc63)#9uxar8u3","timestamp":0},{"desc":"raw(0014f9907afae2c3f8be3bce12dca19be2867245f88d)#7y6gnm25","timestamp":0},{"desc":"raw(001431aacee7ab7794a8766723536c62f05b7f63fc84)#0cf3ks45","timestamp":0},{"desc":"raw(0014c06f7953519dbfd1e25ff18d001be590401ecba6)#lrnv7qcq","timestamp":0},{"desc":"raw(0014434089931c3b52d2def67f62eac99d23058a9f09)#cg0nslav","timestamp":0},{"desc":"raw(0014bf4d1f68349dd65990d6338ac61ca946e98f8cc5)#tx8gf6l3","timestamp":0},{"desc":"raw(001417a66b36201d81e59f48939c67755358164dec66)#4x3h7lv2","timestamp":0},{"desc":"raw(0014064bcbe8d6208bb58e2147a5bbcd97dbdcd298a4)#qd9yj6zk","timestamp":0},{"desc":"raw(0014f2dc57444e57098f00c57060f97da9cc8fadac3c)#f5uwe54v","timestamp":0},{"desc":"raw(00142fb4648a340c06a2f2f9742d2e7832e8f347a2a2)#q6fx2u8j","timestamp":0},{"desc":"raw(0014564a50bddba40328fd6bea63ea434f6e47269333)#reh8v5v0","timestamp":0},{"desc":"raw(001401f256832bf51c8690c921412ee0d89b8dcc5d8d)#l7tyefep","timestamp":0},{"desc":"raw(0014ad3377bab25e7ce5de2d243c2af1a1a744c5ba25)#n8hylkg4","timestamp":0},{"desc":"raw(00146cde66581cc43d654da4d139660fce31dbd27821)#58pr9vpr","timestamp":0},{"desc":"raw(0014d39f838ee2bd8734fb90354d2b15a1c1eb8900ac)#s526lyam","timestamp":0},{"desc":"raw(001444fafb985f591ccfe2b9990e0e75783bb0bde701)#cpmq7gwp","timestamp":0},{"desc":"raw(00145d75454b0038bc70f12306484544038558140938)#90h7e468","timestamp":0},{"desc":"raw(00147a01fddfdee428515cf4c0ac5ac609fd0724620f)#9cejp0ux","timestamp":0},{"desc":"raw(001406eb1337f2a3daf860b7ac8b9b46c10d0f70010e)#mk362vyg","timestamp":0},{"desc":"raw(0014d6622202a9ddbd298e0abdaebcdd1ebf13f43dea)#8j4fmku8","timestamp":0},{"desc":"raw(00148b4e1f7cb1819dd6f3386d3bd3b49f82ec69bbd5)#gap3ycw7","timestamp":0},{"desc":"raw(001462b2fe372c6ca2a39bb453b17bae10310bcd02bb)#ndftc243","timestamp":0},{"desc":"raw(0014b9abb44d5504daff04903f5dcfdbd6bb1613d586)#fu2d8tt2","timestamp":0},{"desc":"raw(0014762aa4cd9edc41765dff328b3662ac7c424ac771)#mh2d5gj8","timestamp":0},{"desc":"raw(00144426d291d198ccd0df68521758afe8cff08743a8)#c40nwl9x","timestamp":0},{"desc":"raw(00140b970dd18df6eaaa790b0050b6929587c47f58ea)#wndmanqc","timestamp":0}]]

@rajarshimaitra
Copy link
Contributor

Interesting.. This should not happen.. Thanks for reporting back.. I will dig through..

@notmandatory
Copy link
Member

FYI I believe core still doesn't support all the miniscript operators in descriptors that bdk supports, which is why we can't use xpub-based descriptors. See https://bitcoindevkit.org/descriptors/#operators.

@rajarshimaitra
Copy link
Contributor

rajarshimaitra commented Aug 31, 2022

I looked into this and reproduced this issue. I probably misunderstood the purpose of start_script_count. Which does not influence the script count at sync time.

Detailing some findings below as fixing this might require some broader discussions.

It seems enforcing script sync number lower than our CACHE_ADDR_BATCH_SIZE (hard coded as 100) is not much straight forward, and can have deeper effect in wallet operations..

The RPC sync always imports the full script pubkey list from the database, defined here.

bdk/src/blockchain/rpc.rs

Lines 387 to 392 in 13cf72f

let scripts_iter = self.ext_spks.iter().chain(&self.int_spks);
if is_descriptor {
import_descriptors(client, start_epoch, scripts_iter)?;
} else {
import_multi(client, start_epoch, scripts_iter)?;
}

Where ext_spks and int_spks are the full list of external and internal descriptor.

bdk/src/blockchain/rpc.rs

Lines 317 to 318 in 13cf72f

let ext_spks = db.iter_script_pubkeys(Some(KeychainKind::External))?;
let int_spks = db.iter_script_pubkeys(Some(KeychainKind::Internal))?;

This can make RPC sync slow as we will be redoing duplicate imports in every sync calls.

And it doesn't seem to be very straight forward either to implement a smaller batch of address for sync only, than our CACHE_ADDR_BATCH_SIZE.

Questions:

  • How to keep track of how many addresses have been imported in the blockchain. Should this be in RpcSyncParams?

  • How can we know when to import new addresses/descriptor into RPC?

  • The sync batch size reduces with CACHE_ADDR_BATCH_SIZE. Should we make that value configurable instead? That seems like the easiest way to solve this particular issue at hand for testing configurations (@casey, @terror you might wanna just use a fork with that value changed, its in src/wallet/mod.rs, until we figure what to do here?? 😅)

But I think we do need to think out something smarter for the RPC blockchain sync. Importing everything for each sync call seems suboptimal.

Tagging @afilini @evanlinjin who might have checked this part of the world recently..

@evanlinjin
Copy link
Member

evanlinjin commented Sep 1, 2022

Thank you @casey for filing this issue, and @rajarshimaitra for identifying the part of the codebase responsible for the inefficient sync logic.

When I was restructuring the RpcBlockchain implementation, I purposefully make a trade-off for reliability over efficiency. This was in part, because of the limitations of the Bitcoin Core JSON-RPC interface and the limitations of the Blockchain trait itself in BDK. The result of this, is having to import all relevant descriptors/scriptPubKeys in every sync cycle.

I propose the following solution:

  1. Make the importmulti/importdescriptors RPC call paginated. I assume this would help us avoid the socket error 35 (probably need to test this theory).
  2. Have an internal "store" that RpcBlockchain uses to keep track of which descriptors/scriptPubKeys are already imported into Bitcoin Core. This store can just be a simple integer that represents derivation index to start import from for next cycle (but should be unique for each Bitcoin Core wallet instance).

Maybe @afilini and @danielabrozzoni can provide a bit more insight/opinions.

PR: #740

P.S. I do not think changing CACHE_ADDR_BATCH_SIZE will solve the issue.

@evanlinjin
Copy link
Member

@terror @casey Would it be possible to show me how you are using RpcBlockchain for testing?

@casey
Copy link
Author

casey commented Sep 3, 2022

@evanlinjin For sure! You can see some of our tests here:

https://github.com/casey/ord/blob/master/tests/wallet.rs

State contains the RpcBlockchain and Wallet:

https://github.com/casey/ord/blob/3e3bdc535cd10bcb9a2962c7776a6f07e4e465ca/tests/state.rs

@notmandatory
Copy link
Member

Based on comments in #740 I'm adding this to the Alpha 1.0.0 milestone.

@notmandatory notmandatory added this to the Alpha 1.0.0 milestone Mar 2, 2023
@notmandatory notmandatory removed this from the 1.0.0-alpha.0 milestone Mar 3, 2023
@MaxFangX
Copy link
Contributor

MaxFangX commented Apr 27, 2023

Just wanted to share that we updated our BDK fork to change CACHE_ADDR_BATCH_SIZE from 100 to 1, and this reduced our total node startup time by over 10x - BDK sync was by far the biggest bottleneck. We use async esplora to sync BDK, and a completely empty wallet was making ~100 API calls to Blockstream Esplora (in batches of 25). Caching addresses doesn't make any sense for us because we expect to generate the wallet seed and update the derivation indexes ourselves, and don't prioritize the import of external seeds as a first-class feature. Ideally this batch size is configurable - is that the case in BDK 1.0? Based on the discussion above it sounds like the current caching strategy might be disposed of entirely in 1.0?

@LLFourn
Copy link
Contributor

LLFourn commented Apr 27, 2023

@MaxFangX indeed. There is no "caching" in BDK 1.0. Script pubkeys are not stored on disk at all. I am quite surprised that the old implementation performs so badly though. It should be pulling in batches of whatever you configure in the esplora client and then stopping after some configurable stop gap iirc.

@notmandatory
Copy link
Member

I think I found something that was slowing down the the pre-1.0 address caching, still WIP but see #985.

@notmandatory notmandatory added bug Something isn't working module-blockchain labels Mar 17, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Mar 17, 2024
@notmandatory
Copy link
Member

This needs to be re-tested with the new 1.0 bitcoind_rpc client.

@LLFourn
Copy link
Contributor

LLFourn commented Mar 17, 2024

I don't think we have a importdescriptors feature at all anymore. you can still do things this way at the application level if you want but there are no guiderails on how to approach using bitcoin core's wallet as your main chain indexing wallet and then BDK to do coin selection/tx construction.

There have been a an attempt by @rajarshimaitra to add this back in the past which would be the best thing to look at to re-introduce this. In general, the problem with the previous appraoch in <1.0 is that it was not using bitcoin rpc as it was intended to beused but instead inserting huge numbers of SPKs into it (hence the issue).

@notmandatory
Copy link
Member

Makes sense. Anyone who ran into this issue please try the shiny new 1.0.0-alpha block by block rpc syncing and let us know if you see any performance issues.

@notmandatory
Copy link
Member

This is fixed in the new bdk_bitcoind_rpc module which doesn't use importdescriptors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working module-blockchain
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants