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

refactor handling of DLC input validation #1

Merged
merged 5 commits into from
Aug 5, 2024

Conversation

conduition
Copy link

  • Removed trailing whitespace (my editor does this automatically so i separated it in the first commit)
  • Refactor handling of inputs for DLC registration:
    • Raise TransactionErrors instead of returning false, to give callers better feedback and to mirror the behavior of other input validation code
    • Avoid duplicating SCT validation logic in mint/dlc.py. Instead rely on the recursive SCT secret verification logic in mint/conditions.py.
  • Rename DLCWitness to SCTWitness, reflecting the fact that this data structure is the witness to an SCT secret (including those which may not be related to DLCs at all)

Copy link
Owner

@lollerfirst lollerfirst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@conduition conduition marked this pull request as draft July 29, 2024 19:47
@conduition
Copy link
Author

Tests are failing, i need some additional time to get unit test runners working on my machine

@conduition
Copy link
Author

I found the cause of my woes: cashubtc#607

…tions

This gives the client an explanation for why the input verification failed.
This aims to make the input validation more succinct and
line up better with existing input validation code.
This commit groups together a bunch of linter error fixes,
some automatic and others (in test_dlc.py) manual.
@conduition
Copy link
Author

conduition commented Aug 5, 2024

I've updated this PR to target the latest commit 6b1d04c in cashubtc#576. The overall changes are the same:

  • Whitespace formatting fixes c58cf9b
  • Rename DLCWitness to SCTWitness 97684a6
  • Raise errors instead of returning false. 42964e7
  • Refactor how DLC input validation is handled 0b5c191
  • [new] Fix a bunch of linter errors 1f287d3 (validated with make check)

I've confirmed tests pass on my machine with make test, though only if I apply cashubtc#607 on top of this PR.

@lollerfirst
Copy link
Owner

lollerfirst commented Aug 5, 2024

Good job. I'm gonna merge this. Unfortunately some tests fail because I fucked up. I changed the migration for the DLC table to have the column settled of type BIT instead of BOOL, as I had read that BOOL is not supported across different DBs while BIT is. Apparently it doesn't like the 0 default value for it.

@lollerfirst lollerfirst merged commit e4463ac into lollerfirst:dlc Aug 5, 2024
10 of 17 checks passed
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

Successfully merging this pull request may close these issues.

2 participants