fix(core-transaction-pool): wallet-manager fallback to database wallet manager findByIndex() when no "local" match

[air1one]

Summary

Resolves #3242

Transaction pool wallet manager : try database wallet manager findByIndex() when local findByIndex() doesn't return a match.

We might not have all existing wallets indexes on transaction pool wallet manager : so in case we get no match we should try with database wallet manager.

Checklist

• Documentation (if necessary)
• Tests (if necessary)
• Ready to be merged

[codecov]

Codecov Report

Merging #3256 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           develop   #3256      +/-   ##
==========================================
+ Coverage    65.18%   65.2%   +0.02%     
==========================================
  Files          431     431              
  Lines        12123   12132       +9     
  Branches      1631    1633       +2     
==========================================
+ Hits          7902    7911       +9     
  Misses        4188    4188              
  Partials        33      33

Impacted Files	Coverage Δ
...ckages/core-transaction-pool/src/wallet-manager.ts	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f028f8...5f0b6a6. Read the comment docs.

[dated]

With these changes the following lines become obsolete, is that right?

core/packages/core-transactions/src/handlers/htlc-refund.ts

Lines 53 to 56 in 8f028f8

	const dbWalletManager: State.IWalletManager = app.resolvePlugin<Database.IDatabaseService>("database")
	.walletManager;
	const lockWallet: State.IWallet = dbWalletManager.findByIndex(State.WalletIndexes.Locks, lockId);

Instead, we can call findByIndex on the wallet manager passed to the throwIfCannotBeApplied method directly.

[dated]

And another question for my personal understanding: should the Wallet Managers be allowed to be out of sync at all?

[spkjp]

@dated Initially the PoolWalletManager has no wallets and clones every database wallet that is missing. The reason why it is an issue for HTLC is because findByIndex is called directly, which doesn't have the fallback to the database wallets as findByAddress/findByPublicKey do. This PR adds the fallback.

With these changes the following lines become obsolete, is that right?

Yes, these should be obsolete now. @air1one

[dated]

Understood! Thanks for the explanation!

[air1one]

With these changes the following lines become obsolete, is that right?

No I think it's better to keep it this way, we actually want to check just against the database wallets, with associated lock transaction forged. (we don't want to validate if the lock transaction is still in the pool and was applied just to tx pool wallet, even though in theory this shouldn't happen)