[DO NOT MERGE YET] New Sigscanner API and full PR check

[timweri]

Summary

This PR does two things:

• Add support for a new Sigscanner API. The old API will still be used as a fallback.
  • The new API will try to verify all commits. Any unverified or failed-to-verify commits will be attempted by the old API. The check passes if, at the end, all commits have been verified.
• The check now verifies all commits in a PR instead of just the head commit.

A few notes

• Since this runs in a PR context, concurrency.cancel-in-progress=true to kill redundant runs when new commits are pushed. If commits are force-pushed off, then users need to manually rerun cancelled runs.
• Each run has 3 limits:
  • Total time: ≤ 10 minutes
  • Total number of commits: ≤ 512. This will block large PRs like big merges.
  • Maximum 3 consecutive errors.

Tried my best to make the bash scripts short. Is it worth parting them out as standalone .sh scripts? I think currently, the complexity is still acceptable.

Having Sigscanner handle multiple commits per call

From the discussion with @erikburt below, it is much more efficient to have Sigscanner handle multiple commits in 1 call. Personally I think it'd be elegant to be able to query Sigscanner by PR to reduce the complexity and the number of network calls in the GHA Workflow.

If possible, I'm inclined to have this iteration accepted and keep this check non-blocking for data collection still. I agree that an enforced version of this workflow should just make 1 call to Sigscanner to scan the PR.

Tests

• All commits are verified: https://github.com/smartcontractkit/sigscanner-test/actions/runs/23563856485
• If both new and old API steps fail, the check fails: https://github.com/smartcontractkit/sigscanner-test/actions/runs/23563596659/job/68609468724
• If new API fails and old API passes, the check passes: https://github.com/smartcontractkit/sigscanner-test/actions/runs/23563529454/job/68609245138
• Merge commit from a branch with unverified commits: https://github.com/smartcontractkit/sigscanner-test/actions/runs/23569301621/job/68628116370?pr=2

[erikburt]

Some PRs get very large and a single push makes this workflow revalidate all previous commits. This seems like a bad idea.

If anything, most this logic should be offloaded to plaid. At least it would be able to maintain some state for already validated commits. Perhaps plaid could maintain/update status checks on a PR as its validating them. So, using webhook events as triggers for plaid-based stateful control-flow rather than orchestrating through stateless GHA workflows executions?

Also curious how that API responds when there’s merge commits present in the list of commits. For example, merging in default branch. But what about merging in another branch with unsigned commits, with a signed merge commit.

Edit: I’m guessing it will return all commits, and now you could easily get to the 512 limit if a 2 month old branch is brought back up to date through a massive merge.

Edit 2: An endpoint that accepts more than one commit in the payload?

[timweri]

Some PRs get very large and a single push makes this workflow revalidate all previous commits. This seems like a bad idea.

If anything, most this logic should be offloaded to plaid. At least it would be able to maintain some state for already validated commits. Perhaps plaid could maintain/update status checks on a PR as its validating them. So, using webhook events as triggers for plaid-based stateful control-flow rather than orchestrating through stateless GHA workflows executions?

Also curious how that API responds when there’s merge commits present in the list of commits. For example, merging in default branch. But what about merging in another branch with unsigned commits, with a signed merge commit.

Edit: I’m guessing it will return all commits, and now you could easily get to the 512 limit if a 2 month old branch is brought back up to date through a massive merge.

Edit 2: An endpoint that accepts more than one commit in the payload?

Plaid does cache if a commit is verified. The design to keep a state per PR in Plaid was decided against to minimize change in Sigscanner. I think having Sigscanner be aware of PRs is reasonable. But functionally, there's no difference. It's between having the complexity (1 PR fetch and a loop) in the action vs in Sigscanner.

Also curious how that API responds when there’s merge commits present in the list of commits. For example, merging in default branch. But what about merging in another branch with unsigned commits, with a signed merge commit.

Merge commits are signed by Github PGP key so they will be accepted. I don't fully understand what you mean by "But what about merging in another branch with unsigned commits, with a signed merge commit."

Edit: I’m guessing it will return all commits, and now you could easily get to the 512 limit if a 2 month old branch is brought back up to date through a massive merge.

This is my main concern regarding the max commit count. I am open to remove this limit if it's infeasible. The main risk here would be DoS of Sigscanner, which has an extremely high throughput. If Github allows that many commits in a PR, Sigscanner should be able to handle it.

Edit 2: An endpoint that accepts more than one commit in the payload?

Is the concern here about the load on Plaid/Sigscanner?

[timweri]

But functionally, there's no difference. It's between having the complexity (1 PR fetch and a loop) in the action vs in Sigscanner.

Well I take it back. The number of round-trip network calls would be reduced from O(|commits|) to O(1)

[timweri]

Okay I get what you mean about the merge commit now. Let me test it out. Ideally it should return all commits.

It does return all commits: https://github.com/smartcontractkit/sigscanner-test/actions/runs/23569301621/job/68628116370?pr=2