Skip to content

feat: add --extraTxs flag to make-release and verify-deployed#11729

Open
martinvol wants to merge 2 commits intorelease/core-contracts/16from
martinvol/extraTXRelease
Open

feat: add --extraTxs flag to make-release and verify-deployed#11729
martinvol wants to merge 2 commits intorelease/core-contracts/16from
martinvol/extraTXRelease

Conversation

@martinvol
Copy link
Copy Markdown
Collaborator

Summary

  • Adds -e / --extraTxs flag to make-release-foundry.sh and verify-deployed-forge.sh
  • Extra transactions JSON file (tracked in releaseData/extraTransactions/) is appended to the end of proposal.json
  • Verification script filters out extra TXs before assertValidProposalTransactions so they don't fail validation

@martinvol martinvol requested a review from a team as a code owner April 22, 2026 15:16
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8f59533fe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +297 to +299
const releaseProposal = proposal.filter(
(tx) => !extraTxs.some((extra) => JSON.stringify(extra) === JSON.stringify(tx))
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restrict extra-tx filtering to appended proposal entries

releaseProposal is built by removing every proposal transaction that deep-equals an entry in extraTxs, regardless of where it appears or how many times it appears. This means any matching transaction outside the appended extra-tx tail (for example, an accidentally duplicated non-release tx in the main release section) is also excluded from assertValidProposalTransactions, so the validation can pass even though the proposal still contains disallowed transactions. Filter by appended position/count (suffix semantics) instead of global value matching.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dded399c1e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +300 to 301
assertValidProposalTransactions(releaseProposal)
assertValidInitializationData(artifacts, proposal, chainLookup, initializationData)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exclude extraTxs from initialization validation

The new --extraTxs flow only removes appended transactions for assertValidProposalTransactions, but assertValidInitializationData still runs on the full proposal. If an appended extra tx uses _setAndInitializeImplementation (for example, for a non-release proxy), verification still fails with missing initialization data or missing artifact errors, which contradicts the script’s documented intent to skip validation for appended extra txs. Pass the filtered proposal (or otherwise subtract extraTxs) into initialization validation as well.

Useful? React with 👍 / 👎.

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.

1 participant