Circleci v2 setup - #6
Merged
Merged
Conversation
Contributor
|
|
ngrinkevich
requested review from
Shadowfiend and
mhluongo
and removed request for
mhluongo
November 16, 2017 00:07
Closed
2 tasks
lionakhnazarov
added a commit
that referenced
this pull request
Jun 4, 2026
…ety) (#4016) ## Context Follow-up to #4000 addressing valid findings from the multi-agent review. Stacks **on top of** `stack/testnet4-02-solidity-logic`; merge after #4000. ## Findings addressed | # | Fix | Commit | |---|-----|--------| | 3 | Allowlist redeploy-safe networks for `EcdsaDkgValidator` (was: mainnet-only denylist) | \`fix(ecdsa/deploy): allowlist redeploy-safe networks\` | | 5 | README documenting vendored `random-beacon-export` regeneration policy (`05_*.js` diverges intentionally) | \`docs(ecdsa/deploy): document vendored random-beacon-export format policy\` | | 7 | Restore \`HardhatUserConfig\` type annotation (TS 4.5 compatible, no \`satisfies\`) | \`fix(ecdsa/hardhat): restore HardhatUserConfig type annotation\` | | 8 | \`verifyOnTenderlyOrContinue\` helper applied to all 4 deploy scripts (was: only 03 swallowed errors) | \`fix(ecdsa/deploy): apply tenderly verify-or-continue helper everywhere\` | | 9 | Comment explaining sepolia named-account role collapse | \`docs(ecdsa/hardhat): note sepolia named-account role collapse\` | | 10 | Comment marking the second \`WalletRegistry.governance()\` read as a deliberate TOCTOU recheck | \`docs(ecdsa/tasks): explain TOCTOU recheck of WR.governance()\` | | extra | Move \`README.md\` out of \`deploy/\` (hardhat-deploy walks the dir and \`require()\`s every file) | \`fix(ecdsa/deploy): move README out of deploy/ dir\` | ## Findings rejected after review - **PR body wording on `WalletRegistry.sol`** (NatSpec-only) — best handled by editing PR #4000's body directly, not as a code commit here. - **Skip-suite disclosure** — same: belongs in PR #4000's body. - \`#4\` (gate \`00_resolve_*\` on env var) — would break \`deployments.fixture()\` in tests (\`yarn test\` runs the hardhat network which needs the skip). - \`#6\` (drop \`approveApplication\` try/catch) — deliberate runtime backstop for forked/aliased networks where artifact ABI ≠ on-chain. ## Test Plan Verified locally with \`FORKING_URL\` unset: - \`cd solidity/ecdsa && yarn test\` → **644 passing, 44 pending, 0 failing** - \`cd solidity/random-beacon && yarn test\` → **535 passing, 397 pending, 0 failing** The 44 + 397 pending are the pre-existing \`describe.skip(...)\` suites (legacy Keep TokenStaking ABI unavailable in current Threshold build).
piotr-roslaniec
added a commit
that referenced
this pull request
Aug 18, 2026
…gate hardening Confirmed bugs and gate gaps from PR #4256 review (decisions #6/#7 plus four related P1 chores), landed in this PR per the same attribution reasoning as the chain-adapter split: - tbtc_redemption.go: convertedEvent.TxMaxFee was assigned from event.TreasuryFee instead of event.TxMaxFee, so every observed redemption event carried the treasury fee as its max fee. - tbtc_dkg.go: validateMemberIndex only checked the upper bound; add chainMemberIndex.Sign() <= 0 so index 0 and negative values are rejected too. - client.yml: pin benchstat to a fixed pseudo-version (was @latest, meaning CI could start failing with no code change); lower the regression gate from +20% to +12% (benchstat already treats ±10% as noise, so +20% let real regressions in the 12-18% band through); add dev to the top-level push trigger and to client-bench's run condition so merges to dev exercise the integration tests and the benchmark gate instead of only main. - ephemeral.UnmarshalPublicKey, tecdsa/{dkg,signing}/protocol.go: the ECDH-time (deferred) unmarshal error used %v, which drops the error chain. Switch to %w and add ephemeral.ErrInvalidPublicKey as a matchable sentinel, so any future retry-policy code can classify the failure with errors.Is instead of parsing the message string. TestGenerateSymmetricKeys_CorruptEphemeralPublicKeyBytes in both packages now asserts errors.Is(err, ephemeral.ErrInvalidPublicKey).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.