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

switch to github actions #544

Merged
merged 2 commits into from Jan 3, 2021
Merged

switch to github actions #544

merged 2 commits into from Jan 3, 2021

Conversation

apoelstra
Copy link
Member

@apoelstra apoelstra commented Jan 3, 2021

See #541 for context.

This was referenced Jan 3, 2021
@sgeisler sgeisler added the ci label Jan 3, 2021
Copy link
Contributor

@sgeisler sgeisler left a comment

Choose a reason for hiding this comment

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

Doesn't really count since the changes are mine, but ACK 😆

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

Looks good. Cr Ack

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

I usually separate the fuzz things into a separate VM because it runs the longest and run the other nighty tests in other VM.
This should reduce the CI time without any cost. What do others think about it?

@apoelstra
Copy link
Member Author

@sanket1729 I think you can do that in another PR, so we can get this one in without changes :) but I agree it's a good idea.

@apoelstra
Copy link
Member Author

Ok, implicit ACKs from me and @sgeisler because we made the PR, an actual ACK from stevenroose on #541, and acks from Sanket and Dr Orlovsky which don't count. Works for me.

@apoelstra apoelstra merged commit 8730b8c into master Jan 3, 2021
@apoelstra apoelstra deleted the 2021-01--gh-actions branch January 3, 2021 21:10
@apoelstra apoelstra mentioned this pull request May 18, 2023
Davidson-Souza pushed a commit to Davidson-Souza/rust-bitcoin that referenced this pull request Jul 12, 2023
Currently we have a wildcard on safety requirements, saying more or less
"plus a bunch of other stuff we don't mention". This is not helpful.

Attempt to fully describe the safety requirements of creating a context
from a raw context (all, signing only, and verification only).

Fix: rust-bitcoin#544
Davidson-Souza pushed a commit to Davidson-Souza/rust-bitcoin that referenced this pull request Jul 12, 2023
…ents

e705bcf Fully describe safety requirements (Tobin C. Harding)

Pull request description:

  Currently we have a wildcard on safety requirements, saying more or less "plus a bunch of other stuff we don't mention". This is not helpful.

  Attempt to fully describe the safety requirements of creating a context from a raw context (all, signing only, and verification only).

  Fix: rust-bitcoin#544

  ## Note

  This is best effort only, will require some thought to review. To do this I read https://doc.rust-lang.org/reference/behavior-considered-undefined.html and then I flicked through `depend/secp256k1/src/secp256k1.c` and `util.h` to look for things that could cause things in the linked to list of UB.

ACKs for top commit:
  apoelstra:
    ACK e705bcf
  Kixunil:
    ACK e705bcf

Tree-SHA512: 0180d196f6d528e3c7a06da54ef58d015df19c351d98030453ae5c5e62e0565797b06169f27f5d8b40ea0b9adba377cadd45dd306c8213d0bdc98b20651766c7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants