Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify chainntfs in case of disapproved blocks #69

Open
matheusd opened this issue Dec 16, 2019 · 1 comment
Open

Verify chainntfs in case of disapproved blocks #69

matheusd opened this issue Dec 16, 2019 · 1 comment

Comments

@matheusd
Copy link
Member

The upstream chainntf package doesn't care about transactions that were reversed due to being in disapproved blocks, due to this mechanic not existing in bitcoin.

Generally we need to verify that the chainnfs package is behaving correctly when disapproved blocks are encountered. And more specifically:

  • Ensure no < 2-conf spend/confirm notifications are requested in ordinary usage;
  • Ensure that txs that trigger the start of a spend/confirm notification callback and are later found to be disapproved stop/reset the notification callback timer appropriately.
@matheusd
Copy link
Member Author

matheusd commented Jan 24, 2020

Findings so far (based on the future v0.3 with PR #74 applied):

  • Most places handling confirm/spend requests don't even handle reorgs right now.

  • Confirmation count for RegisterConfirmationsNtfn() is 1-based (i.e. numConfs == 1 means the block where the script/outpoint is mined, numConfs == 2 means the next block, etc - source)

  • Spend notifications are sent at exactly the block where the spend occurred

  • For (non-test) RegisterConfirmationsNtfn() uses:

    • fundingmanager.go uses parametrized (by server.go#NumRequiredConfs) number of confirmations between 3 and 6
    • utxonursery.go uses parametrized (by ConfDepth) number of confirmations but is currently hard-coded (server.go#ConfDepth) to 1
    • breacharbiter.go uses hard-coded confirmation depth of 1 for breach and justice txs
    • contractcourt/ uses hard-coded confirmation depth of 1 when watching for the generated resolving txs
    • peer.go uses hard-coded confirmation depth of 1 to watch for cooperative close txs
  • For RegisterSpendNtfn():

    • chainwatcher.go doesn't handle reorgs but assumes once a channel close tx is broadcast the channel will never work again.
    • htlc_outgoing_contest_resolver.go waits for a spend to find out a preimage so it's reorgs are inconsequential (once the node learns the preimage it can't be taken back)
    • htlc_timeout_resolver.go waits for either a spend with a preimage or an htlc timeout
    • htlc_success_resolver.go` waits for a second-level htlc tx
    • sweeper watches for spends of any outputs it's attempting to sweep so it can remove them from the list of attempted sweeps
    • breacharbiter.go watches for spends of breached outputs to be able to send justice txs of second-level sweeps made by the remote party

Alternatives for handling disapproved blocks:

For confirmation ntfn requests:

  • Change call sites to use a confirmation height of at least 2
  • Modify txnotifier to notice disapproved blocks and reset the confirmation height
  • Return errors when callers attempt to register for confirmation count < 2
  • Verify timeouts for htlcs and local commit script outputs to ensure the extra block won't cause a risk to remote parties in breach scenarios
  • (alternative) modify call sites to handle the NegativeConf event channel (ideal in that it handles >2 reorgs but less important for decred)
  • (alternative) add a ProbablyDone event channel to ConfirmationEvent such that callers receive on it instead of Confirmed and the ProbablyDone channel is sent to once the txnotifier assumes there's little risk of reorg. This would entail adding a heuristic to the txnotifier such that it can calculate this probability. Probably overkill.

For spend ntfn requests:

  • Modify the txnotifier to send spend notifications only on the next block after a spend is detected as long as the next block does not disapprove the regular txs
  • Verify all relevant timelocks are of at least 2 blocks in the future
  • (alternative) modify call sites to handle the Reorg chan
  • (alternative) same as above with ProbablyDone event channel

@matheusd matheusd mentioned this issue Jan 24, 2020
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant