-
Notifications
You must be signed in to change notification settings - Fork 168
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
[RFC] CI for Ensuring Deterministic Transaction Generation and Stability #590
Comments
Thanks to @dbast we already have CI running our test suite on each PR update. And we have this test: https://github.com/SeedSigner/seedsigner/blob/dev/tests/test_decodepsbtqr.py#L216 That test wasn't really meant to be a rigorous signature checker, but since the expected result is hard-coded, it's effectively serving the purpose you describe above. However, it would be better to write an explicit standalone test (in tests/test_seed.py?). Even better if the test data uses a test vector from, say, Bitcoin Core's test suite. |
Could even call it something like "test_deterministic_signatures_never_change.py" so that we'd all be up-in-arms on any pr that deletes lines from the test. |
Also note that such a test also belongs in I think it makes sense to have redundant tests in BOTH SeedSigner and So if no such test exists in |
this also makes the CI results on Github more trustable #568 as plain action versions are git tags and thus are mutable = can influence the workflow outcome if manipulated.... (another step on top would be installing dependencies during CI via poetry lock files instead of requirement*.txt files, to validate the sha256 of all installed dependencies during CI runs) |
I don't have any experience with Poetry but am interested to learn more. Its (optional) compatibility with our existing requirements.txt seems, at a minimum, an easy, obvious win. Dunno how much it makes sense to keep a toe in both worlds or to just rip the bandaid off and do a full switch. But @dbast let's pull this out into its own convo / PR. |
Alternatively, there's uv. I've been experimenting with it and having good results. It generates a |
Following the recent chat on X.com about 'Dark Skippy' - the sneaky exfiltration of master secret seeds by dodgy signing devices - I'm wondering if there's any interest in a Continuous Integration (CI) job. This job would use seedsigner's code to sign a transaction, and check that the signature doesn't change when a Pull Request is merged.
The idea here is to help prevent attacks where malware might be snuck into the codebase through a series of seemingly harmless pull requests. These could look like small refactors or new features, but once they're all merged, they might do something we don't want.
I'd be more than willing to assist in the implementation of this feature, but I wanted to gauge interest and ensure I'm not overlooking any potential concerns or complications.
What do you think?
The text was updated successfully, but these errors were encountered: