`ord wallet inscribe` takes longer with each inscription

[Jamil]

We're running a hosted service for inscriptions, and we've done around ~2,000 inscriptions with ord. It seems like though ord wallet send is pretty quick, each ord wallet inscribe takes longer for each call. On a wallet which has made 50 inscriptions, it takes ~1.5 seconds, and on a wallet on a fresh node which has made ~2,000 inscriptions, it takes 15 seconds per inscription.

in wallet/inscribe.rs, there are a few maps and loops that seem to be linear with the number of inscriptions made or UTXOs received, so I'm wondering if that's making these calls be linear with the "use" of the wallet for inscriptions. happy to get our team to help make PRs to make optimizations to this (if those are indeed the issue), but would appreciate any guidance on what might be the bottlenecks here, or if anyone's experienced this before

again, happy to contribute to helping on the performance issues, just would be great to have some context on what might be the bottlenecks with ord wallet inscribe

[Psifour]

There are a fair number of optimizations that could be made there. My personal recommendation would be the inclusion of an optional flag that disables the majority of the more intensive checks (like https://github.com/casey/ord/blob/f599b82a93e90faa8937cc1ea7008261d457df50/src/subcommand/wallet/inscribe.rs#L169). While these are VERY useful for the average user (and worth the overhead), the ability to easily bypass them when working in batch operations would likely result in performance increases.

[Jamil]

After a bit of digging, it looks like interestingly:

$ time ./bin/ord wallet inscribe test.txt --dry-run --satpoint [SATPOINT] --fee-rate 25
…

real	0m3.457s
user	0m0.758s
sys	0m0.053s

$ time ord wallet inscribe test.txt --satpoint [SATPOINT] --fee-rate 25
…

real	0m16.081s
user	0m0.790s
sys	0m0.053s

So the difference between running with --dry-run and without is ~13 seconds, almost none of which is actually process/user time. The only difference between running with --dry-run and without is backing up the recovery key, and then signing and sending the transactions.

[Jamil]

^ that also means that it's not gonna be stuff like those loops or anything to do with the inefficiencies in the code, the only difference between running with --dry-run and without is

      if !self.no_backup {
        Inscribe::backup_recovery_key(&client, recovery_key_pair, options.chain().network())?;
      }

      let signed_raw_commit_tx = client
        .sign_raw_transaction_with_wallet(&unsigned_commit_tx, None, None)?
        .hex;

      let commit = client
        .send_raw_transaction(&signed_raw_commit_tx)
        .context("Failed to send commit transaction")?;

      let reveal = client
        .send_raw_transaction(&reveal_tx)
        .context("Failed to send reveal transaction")?;

[Jamil]

Ahh writing up the above comment led to the answer: it's inefficiencies in backing up the recovery keys. Calling with --no-backup results in the same execution time for --dry-run and without. Backing up every key in the descriptors file is causing the issue.

If the backup isn't required after the reveal transaction is mined, I assume the best solution would be to remove it once it's no longer needed.

[Psifour]

Glad I didn't prematurely optimize this. Could this still be a strong case for a batch mode that stores these in memory and does the backup when leaving batch mode?

[Jamil]

oops hit the wrong button there and accidentally closed the issue

yeah, can imagine something like that, or if we don't need to store it in the descriptors there might be a case to be made to do the backup in some O(1) way somewhere else, or if possible "throwing away" the recovery key when the reveal transaction is confirmed (since presumably we don't need it anymore then)

[raphjaph]

We were very paranoid about transaction construction and broadcasting so we have many checks and fallbacks. This blew up faster than expected so we're still trying to scale up dev work. Tomorrow we are reviewing PRs and doing some coding on Discord if you guys want to join. https://discord.gg/fbF3AZC7?event=1072606762097725580

Otherwise keep opening PRs and Issues, we'll try to get to them!

[Jamil]

Thanks for all your hard work @raphjaph, and totally makes sense to have strict defaults! Will definitely join, would love to help jump on some issues

[jack-linden]

Hey @raphjaph @Psifour

Was hoping to resurrect attention to this issue. We continue to see performance degradation over time even with the --no-backup flag.

The slowness appears to result from the call to pub(crate) fn get_inscriptions used both by the inscriptions subcommand but also as part of the inscribe flow:

  pub(crate) fn run(self, options: Options) -> Result {
    let inscription = Inscription::from_file(options.chain(), &self.file)?;

    let index = Index::open(&options)?;
    index.update()?;

    let client = options.bitcoin_rpc_client_for_wallet_command(false)?;

    let mut utxos = index.get_unspent_outputs(Wallet::load(&options)?)?;

    let inscriptions = index.get_inscriptions(None)?; 

On our node which has processed only ~3k inscriptions we see this particular function take 5-10 seconds per call.

[jack-linden]

Based on what I've gathered from its usage in impl Inscribe, it seems excessive to load all inscriptions into memory just to check that the selected cardinal UTXOs don't contain inscriptions.

This is especially the case for when inscribing with an explicit satpoint (e.g., --satpoint) where we should only need to cross-check that UTXO against the index.

[raphjaph]

We've fixed it in fb0b1e826df34de0f6d8d6527c617d2e8d32c0ac. Should be usable now.