fix(watch-only): make AddressCache::new fallible and replace panics with typed errors

[Sandipmandal25]

Description and Notes

Closes part of #463

AddressCache::new and AddressCacheInner::new previously panicked on database load failures. This PR makes both fallible.

While at it the AddressCacheDatabase trait’s save() and update() methods were made fallible too they were silently panicking inside implementations.

What changed

• AddressCacheDatabase::save() and update() now return Result<(), Self::Error>
• Added three new WatchOnlyError variants: PoisonedLock, CachedAddressNotFound, IndexOutOfBounds
• All public methods that had expect("poisoned lock") on the RwLock now propagate via map_err(|_| WatchOnlyError::PoisonedLock)?
• maybe_derive_addresses made fallible and now propagates errors instead of swallowing them
• kv_database.rs: fixed three bare unwraps (item.value(), serde_json::to_vec, Txid::from_slice), added InvalidTxid variant
• memory_database.rs: get_transaction was returning PoisonedLock when tx wasn’t found — fixed to TransactionNotFound
• Updated call sites in florestad.rs, json_rpc/server.rs, json_rpc/blockchain.rs, electrum_protocol.rs

Verification

cargo test -p floresta-watch-only

new_returns_err_when_database_load_fails triggers a real failure by writing invalid bytes directly into the KV bucket (no stub).

[jaoleal]

Closes part of #463 and Competency Test for sob 2026 to be submitted along with proposal

Just to be clear that was my mistake, a competency test ideally should not be a PR. Thats what i wrote on our discord channel: https://discord.com/channels/1185232004506198056/1488947854612365353/1489335077769838685

[jaoleal]

When i started reading the code here i noticed that this is partially addressed on #804 but i have the intuition that it will be discarded, AFAIK the approach there was already reviewed and validated, maybe you could steal the code there and implement it here ? (That is, the error handling for the watch only interface)

[Sandipmandal25]

When i started reading the code here i noticed that this is partially addressed on #804 but i have the intuition that it will be discarded, AFAIK the approach there was already reviewed and validated, maybe you could steal the code there and implement it here ? (That is, the error handling for the watch only interface)

hey @jaoleal when you say "it will be discarded" do you mean pr #804 ? also by "steal the code from #804" and should i just take the full lib.rs error handling changes from there?

[Sandipmandal25]

Closes part of #463 and Competency Test for sob 2026 to be submitted along with proposal

Just to be clear that was my mistake, a competency test ideally should not be a PR. Thats what i wrote on our discord channel: https://discord.com/channels/1185232004506198056/1488947854612365353/1489335077769838685

i missed this acknowledging my mistake.

[jaoleal]

hey @jaoleal when you say "it will be discarded" do you mean pr #804 ?

Yes, Im not certain what will be discarded but for sure the error handling for the AddressCache is desirable.

also by "steal the code from #804" and should i just take the full lib.rs error handling changes from there?

Yes, but only if you think its proper.

[Sandipmandal25]

hey @jaoleal when you say "it will be discarded" do you mean pr #804 ?

Yes, Im not certain what will be discarded but for sure the error handling for the AddressCache is desirable.

also by "steal the code from #804" and should i just take the full lib.rs error handling changes from there?

Yes, but only if you think its proper.

i willl expand this with the full lib.rs error handling changes.

[Sandipmandal25]

hey @jaoleal when you say "it will be discarded" do you mean pr #804 ?

Yes, Im not certain what will be discarded but for sure the error handling for the AddressCache is desirable.

also by "steal the code from #804" and should i just take the full lib.rs error handling changes from there?

Yes, but only if you think its proper.

Notable changes on 2e98ef2

save and update on AddressCacheDatabase now return Result.

Removed all poisoned lock panics in public methods errors are now propagated instead.

memory_database was returning the wrong error when a transaction was not found fixed.

Removed remaining unwraps in kv_database, better error handling overall.

also the pr desc is changed accordingly.

[Micah-Shallom]

noticed this pattern repeat itself multiple times across your change..how about wrapping it for read and write ...so we can easily do a let inner = self.read()? or let inner = self.write()?

[Sandipmandal25]

yeah addedread_inner / write_innerhelpers, same pattern as memory_database already had.

[Micah-Shallom]

|_| matches the PoisonError and throws it away...when this gets propagated upwards, what the user will see is Error: Poison Lock error...we cant't tell from which of the numerous call sites this was triggered from...WYT?

[Sandipmandal25]

i think poisonError doesnt carry call site info, just the guard backtrace gives the location. same pattern in memory_database.rs

[jaoleal]

It appears that the second commit is overwriting the first one.

[Sandipmandal25]

It appears that the second commit is overwriting the first one.

squashing into one commit!

[luisschwab]

Needs rebase

[Sandipmandal25]

Needs rebase

rebased!

[moisesPompilio]

You could just import Error::Blockchain, and if possible, it would be better to add automatic handling using the impl_error_from trait here.

[Sandipmandal25]

Error::Blockchain takes a boxed trait object so impl_error_from wont work directly since it doesnt box. also the error type here is generic. do you have anything specific in mind or is just the import fine?

[moisesPompilio]

I think passing FromSliceError as an InvalidTxid error may be wrong, because FromSliceError is not exclusive to txid.

[Sandipmandal25]

will remove the variant and used expect instead.