search

SiaFoundation / coreutils

Implementations of core Sia components
MIT License
1 stars 4 forks source link

Persist locked outputs in the wallet #35

Open mike76-dev opened 8 months ago

mike76-dev commented 8 months ago

The outputs in w.locked map are not persisted. If the wallet is shut down after a transaction is broadcast and then restarted immediately (before the next block is mined), the wallet may try to spend an already spent output, because it's unaware.

n8maninger commented 8 months ago

I'm not entirely against letting the Store implementation handle the locking of UTXOs, but it adds additional IO overhead with minimal benefit. The UTXOs will eventually be included in a block or the tpool will pick them back up.

mike76-dev commented 8 months ago

Fair enough, but this is also why I'd implement my own wallet rather than use the suggested implementation. In my use case, a double-spending means a failed benchmark for a host, and while it's not a critical failure, I'd rather avoid it.

n8maninger commented 8 months ago

That would only be a practical concern if you frequently restart your contract formation service. I recommend trying it out in production before re-implementing. Sia Central has used a similar wallet implementation with no issues.

mike76-dev commented 8 months ago

Also true.

ChrisSchinnerl commented 8 months ago

@n8maninger how about we start with extending the interface of the store to support this and have implementations decide whether to implement this in memory or somewhere else? That way the implementation becomes more reusable in the short term.

Long term I don't think it would be too bad to persist outputs. Especially for the host it might be nice to be able to update without potentially losing the ability to form contracts for 10 minutes.