decentralized-identity / sidetree

Sidetree Specification and Reference Implementation
https://identity.foundation/sidetree/spec
Apache License 2.0
438 stars 112 forks source link

If LockMonitor unable to broadcast create lock txn, then the lock can become invalid #753

Closed mudiali closed 4 years ago

mudiali commented 4 years ago
  1. Assume the valid duration for a lock is between 90 to 100 blocks.
  2. Let's say that the current block height is 200 and the above function created a lock transaction which unlocks at block 295. This means that the lock duration is295 - 200 = 95 blocks which is valid. Once the transaction is created & signed, it is saved in the lock DB and broadcasted.
  3. Let's say that the broadcast fails (network congested or satoshi client is down or the node crashed) until the current block is 211. The LockMonitor just tries to rebroadcast until it is successful.
  4. The node is finally able to broadcast and the transaction actually gets written in block 211.
  5. With the unlock block being 295, the lock duration is 295 - 211 = 84 which is considered invalid.

In the above case, the following line will throw a sidetree error with code lock_resolver_duration_is_invalid and the lock monitor will never be able to resolve resulting in a hung periodic poll loop. The currently coded LockMonitor will not release the lock either.

Line that will throw: https://github.com/decentralized-identity/sidetree/blob/8fa9f21e649a663aa771a23e81329f3dce56ef25/lib/bitcoin/lock/LockMonitor.ts#L214

Suggested solution

  1. Maybe catch the above error, and just call this.releaseLock(...) to return the back to the wallet and let the next iteration to retry creating the lock?
  2. If a transaction was never broadcasted (or broadcasted and is in the mempool) then maybe remove it from the mempool?
  3. ??
csuwildcat commented 4 years ago

Resolution: try using relative lock