Skip to content

potential fixes for tempo deployments#1751

Draft
0xDEnYO wants to merge 1 commit intomainfrom
tempo-fixes
Draft

potential fixes for tempo deployments#1751
0xDEnYO wants to merge 1 commit intomainfrom
tempo-fixes

Conversation

@0xDEnYO
Copy link
Copy Markdown
Contributor

@0xDEnYO 0xDEnYO commented Apr 29, 2026

Which Linear task belongs to this PR?

Why did I implement it this way?

Checklist before requesting a review

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

@lifi-action-bot lifi-action-bot marked this pull request as draft April 29, 2026 11:16
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Walkthrough

This PR adds Tempo network support across deployment and task scripts. Changes include adding Tempo network configuration to config/networks.json, creating a Tempo Foundry profile in foundry.toml, introducing helper functions for Tempo detection and CLI flag generation in script/helperFunctions.sh, and updating multiple deployment and task scripts to dynamically apply Tempo-specific configuration flags.

Changes

Cohort / File(s) Summary
Configuration
config/networks.json, foundry.toml
Added Tempo network deploy flags and created dedicated Foundry profile for Tempo with fee-token configuration and verification endpoint adjustment.
Helper Functions
script/helperFunctions.sh
Introduced four new functions: isTempoNetwork(), getTempoForgeProfilePrefix(), getForgeLegacyCliFlag(), and getTempoFeeTokenCliFlag() to detect Tempo networks and generate appropriate CLI flags.
Deployment Scripts
script/deploy/deployAndStoreCREATE3Factory.sh, script/deploy/deploySingleContract.sh, script/deploy/deployUpgradesToSAFE.sh
Updated forge script and cast send commands to dynamically compute and apply Tempo profile prefix and legacy/fee-token flags per network, replacing hardcoded --legacy usage.
Task Scripts
script/tasks/acceptOwnershipTransferPeriphery.sh, script/tasks/checkExecutorAndReceiver.sh, script/tasks/diamondUpdateFacet.sh, script/tasks/updateFacetConfig.sh
Modified forge script command construction to include computed Tempo profile prefix and dynamically determined legacy CLI flag instead of unconditional --legacy option.
Transaction Utilities
script/universalCast.sh
Updated universalSendRaw and universalSendValue functions to apply Tempo profile prefix and fee-token/legacy flags dynamically to cast send commands.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

AuditNotRequired, NewNetwork

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description contains only the template structure with unchecked checkboxes and no substantive content; the 'Why did I implement it this way?' section is completely empty. Fill in the Linear task reference, explain the implementation rationale for Tempo network support across deployment scripts, and complete the relevant checklist items to indicate what has been reviewed and tested.
Title check ❓ Inconclusive The title 'potential fixes for tempo deployments' is vague and generic, using imprecise language ('potential fixes') that doesn't clearly convey the specific changes made. Replace with a more specific title that clearly describes the main change, such as 'Add Tempo network support to deployment and transaction scripts' or 'Enable Tempo network deployment configurations'.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 86.67% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tempo-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
script/deploy/deployUpgradesToSAFE.sh (1)

38-39: Scope the Tempo helper outputs locally.

These are just per-invocation temporaries, so leaving them non-local needlessly leaks state inside a sourced bash environment.

♻️ Suggested cleanup
-    TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
-    LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
+    local TEMPO_PROFILE_PREFIX
+    TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
+    local LEGACY_CLI_FLAG
+    LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
As per coding guidelines, "Use local keyword when declaring variables inside functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/deploy/deployUpgradesToSAFE.sh` around lines 38 - 39, Two temporaries
(TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG) are being set without local scoping;
change their declarations inside the function so they are local variables. Use
local when assigning the outputs of getTempoForgeProfilePrefix and
getForgeLegacyCliFlag (i.e., make TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG
local) to avoid leaking state into the sourced shell environment.
script/tasks/checkExecutorAndReceiver.sh (1)

59-60: Make the new Tempo CLI vars local here too.

These names only serve the current loop iteration, so keeping them global adds avoidable shared state in a sourced task script.

♻️ Suggested cleanup
-    TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
-    LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
+    local TEMPO_PROFILE_PREFIX
+    TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
+    local LEGACY_CLI_FLAG
+    LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
As per coding guidelines, "Use local keyword when declaring variables inside functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/tasks/checkExecutorAndReceiver.sh` around lines 59 - 60, The variables
TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG are currently being set as globals but
are only needed per-iteration; change their declarations to be local in the
scope where they’re assigned (the loop or function that calls
getTempoForgeProfilePrefix and getForgeLegacyCliFlag) so they do not pollute
shared state—i.e., locate the assignment lines for TEMPO_PROFILE_PREFIX and
LEGACY_CLI_FLAG and prefix them with the local keyword so they are scoped to the
current iteration/function.
script/helperFunctions.sh (1)

3822-3867: Complete the new helper docblocks with Args and example usage.

The new comments already include description/usage/returns, but the repo helper-doc format for modified helpers expects explicit Args and example lines too.

As per coding guidelines ".cursor/rules/300-bash.mdc: Follow the helper function documentation format (brief description, Usage, Args, Returns, examples) when modifying/adding functions like Tempo network predicates and CLI-flag generators."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/helperFunctions.sh` around lines 3822 - 3867, Add missing "Args" and
example usage lines to each helper docblock so they follow the repo helper-doc
format: for isTempoNetwork, document Args: NETWORK (string) and give a one-line
example showing invocation; for getTempoForgeProfilePrefix,
getForgeLegacyCliFlag, and getTempoFeeTokenCliFlag, add Args: NETWORK (string)
and an Examples line showing how the function is called and how the returned
string is used (e.g., concatenated before a CLI invocation). Update the comment
sections above the functions isTempoNetwork, getTempoForgeProfilePrefix,
getForgeLegacyCliFlag, and getTempoFeeTokenCliFlag to include these "Args" and
"Examples" entries consistent with ".cursor/rules/300-bash.mdc".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@script/deploy/deployAndStoreCREATE3Factory.sh`:
- Line 60: The primary forge invocation in
script/deploy/deployAndStoreCREATE3Factory.sh (the long command string starting
with "${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge script
script/deploy/facets/DeployCREATE3Factory.s.sol ...") is missing
TEMPO_FEE_CLI_FLAG while the fallback invocation includes it; modify that
primary command to insert ${TEMPO_FEE_CLI_FLAG} into the same position used in
the fallback (i.e. include the TEMPO_FEE_CLI_FLAG variable alongside the other
injected flags before ${LEGACY_CLI_FLAG}) so the dynamically injected fee flag
is applied consistently across both branches.

In `@script/tasks/acceptOwnershipTransferPeriphery.sh`:
- Around line 53-64: The broadcast command built for
AcceptOwnershipTransferPeriphery.s.sol omits the Tempo fee-token flag declared
in config/networks.json, so update the command construction used by
executeAndParse (and the corresponding cast fallback) to append the Tempo
fee-token flag alongside TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG; specifically,
modify the line that constructs the forge script invocation inside the loop (the
string passed to executeAndParse) to include the fee-token flag (the same token
configured per-network) and ensure any alternate/fallback command path where
executeAndParse falls back to cast also receives the identical flag so
governance routing/fee-token behavior remains consistent.

In `@script/tasks/updateFacetConfig.sh`:
- Around line 81-88: updateFacetConfig() constructs the forge/cast broadcast
command but omits network-specific custom flags (Tempo's --tempo.fee-token)
because it never reads or injects customDeployFlags; fix by fetching and
appending the network's custom deploy flags into the constructed command used by
executeAndParse (alongside TEMPO_PROFILE_PREFIX, LEGACY_CLI_FLAG,
SKIP_SIMULATION_FLAG, GAS_ESTIMATE_MULTIPLIER) so the --tempo.fee-token from
config/networks.json is included for the `forge script "$SCRIPT_PATH" --fork-url
"$NETWORK" --json --broadcast ...` path and mirror the same injection into the
cast fallback branch; reference getTempoForgeProfilePrefix,
getForgeLegacyCliFlag, customDeployFlags, updateFacetConfig(), and
executeAndParse when making the change.

---

Nitpick comments:
In `@script/deploy/deployUpgradesToSAFE.sh`:
- Around line 38-39: Two temporaries (TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG)
are being set without local scoping; change their declarations inside the
function so they are local variables. Use local when assigning the outputs of
getTempoForgeProfilePrefix and getForgeLegacyCliFlag (i.e., make
TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG local) to avoid leaking state into the
sourced shell environment.

In `@script/helperFunctions.sh`:
- Around line 3822-3867: Add missing "Args" and example usage lines to each
helper docblock so they follow the repo helper-doc format: for isTempoNetwork,
document Args: NETWORK (string) and give a one-line example showing invocation;
for getTempoForgeProfilePrefix, getForgeLegacyCliFlag, and
getTempoFeeTokenCliFlag, add Args: NETWORK (string) and an Examples line showing
how the function is called and how the returned string is used (e.g.,
concatenated before a CLI invocation). Update the comment sections above the
functions isTempoNetwork, getTempoForgeProfilePrefix, getForgeLegacyCliFlag, and
getTempoFeeTokenCliFlag to include these "Args" and "Examples" entries
consistent with ".cursor/rules/300-bash.mdc".

In `@script/tasks/checkExecutorAndReceiver.sh`:
- Around line 59-60: The variables TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG are
currently being set as globals but are only needed per-iteration; change their
declarations to be local in the scope where they’re assigned (the loop or
function that calls getTempoForgeProfilePrefix and getForgeLegacyCliFlag) so
they do not pollute shared state—i.e., locate the assignment lines for
TEMPO_PROFILE_PREFIX and LEGACY_CLI_FLAG and prefix them with the local keyword
so they are scoped to the current iteration/function.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 572bed5e-2a88-4704-acbc-9014fedb8b70

📥 Commits

Reviewing files that changed from the base of the PR and between 81a22e2 and aeb1e0f.

📒 Files selected for processing (11)
  • config/networks.json
  • foundry.toml
  • script/deploy/deployAndStoreCREATE3Factory.sh
  • script/deploy/deploySingleContract.sh
  • script/deploy/deployUpgradesToSAFE.sh
  • script/helperFunctions.sh
  • script/tasks/acceptOwnershipTransferPeriphery.sh
  • script/tasks/checkExecutorAndReceiver.sh
  • script/tasks/diamondUpdateFacet.sh
  • script/tasks/updateFacetConfig.sh
  • script/universalCast.sh

# 1) Try forge script (works for chains in Foundry's alloy-chains list)
if executeAndParse \
"PRIVATE_KEY=\"$PRIVATE_KEY\" forge script script/deploy/facets/DeployCREATE3Factory.s.sol --fork-url \"$NETWORK\" --json --broadcast --legacy --slow $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
"${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge script script/deploy/facets/DeployCREATE3Factory.s.sol --fork-url \"$NETWORK\" --json --broadcast ${LEGACY_CLI_FLAG}--slow $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Apply TEMPO_FEE_CLI_FLAG to the primary forge script path as well.

At Line 60, TEMPO_FEE_CLI_FLAG is not included, while the fallback at Line 91 includes it. This makes Tempo behavior path-dependent.

💡 Suggested patch
-    "${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge script script/deploy/facets/DeployCREATE3Factory.s.sol --fork-url \"$NETWORK\" --json --broadcast ${LEGACY_CLI_FLAG}--slow $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
+    "${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge script script/deploy/facets/DeployCREATE3Factory.s.sol --fork-url \"$NETWORK\" --json --broadcast ${TEMPO_FEE_CLI_FLAG}${LEGACY_CLI_FLAG}--slow $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \

As per coding guidelines ".cursor/rules/300-bash.mdc: ensure dynamically injected flags ... are applied consistently across all branches".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge script script/deploy/facets/DeployCREATE3Factory.s.sol --fork-url \"$NETWORK\" --json --broadcast ${LEGACY_CLI_FLAG}--slow $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
"${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge script script/deploy/facets/DeployCREATE3Factory.s.sol --fork-url \"$NETWORK\" --json --broadcast ${TEMPO_FEE_CLI_FLAG}${LEGACY_CLI_FLAG}--slow $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/deploy/deployAndStoreCREATE3Factory.sh` at line 60, The primary forge
invocation in script/deploy/deployAndStoreCREATE3Factory.sh (the long command
string starting with "${TEMPO_PROFILE_PREFIX}PRIVATE_KEY=\"$PRIVATE_KEY\" forge
script script/deploy/facets/DeployCREATE3Factory.s.sol ...") is missing
TEMPO_FEE_CLI_FLAG while the fallback invocation includes it; modify that
primary command to insert ${TEMPO_FEE_CLI_FLAG} into the same position used in
the fallback (i.e. include the TEMPO_FEE_CLI_FLAG variable alongside the other
injected flags before ${LEGACY_CLI_FLAG}) so the dynamically injected fee flag
is applied consistently across both branches.

Comment on lines +53 to +64
local TEMPO_PROFILE_PREFIX
TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$CURRENT_NETWORK")
local LEGACY_CLI_FLAG
LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$CURRENT_NETWORK")

# execute script
attempts=1

while [ $attempts -lt 11 ]; do
# Execute, parse, and check return code
executeAndParse \
"NETWORK=$CURRENT_NETWORK FILE_SUFFIX=$FILE_SUFFIX forge script script/tasks/solidity/AcceptOwnershipTransferPeriphery.s.sol --fork-url $CURRENT_NETWORK --json --broadcast --verify --skip-simulation --legacy --tc DeployScript" \
"${TEMPO_PROFILE_PREFIX}NETWORK=$CURRENT_NETWORK FILE_SUFFIX=$FILE_SUFFIX forge script script/tasks/solidity/AcceptOwnershipTransferPeriphery.s.sol --fork-url $CURRENT_NETWORK --json --broadcast --verify --skip-simulation ${LEGACY_CLI_FLAG}--tc DeployScript" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This Tempo broadcast path still omits the fee-token flag.

Line 64 adds the profile/legacy handling, but not the Tempo fee-token declared in config/networks.json Line 1284. Since this command broadcasts and verifies directly, Tempo ownership transfers here can still fail for the same reason as before.

🔧 Suggested fix
     local TEMPO_PROFILE_PREFIX
     TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$CURRENT_NETWORK")
     local LEGACY_CLI_FLAG
     LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$CURRENT_NETWORK")
+    local TEMPO_FEE_CLI_FLAG
+    TEMPO_FEE_CLI_FLAG=$(getTempoFeeTokenCliFlag "$CURRENT_NETWORK")
 
     # execute script
     attempts=1
@@
       # Execute, parse, and check return code
       executeAndParse \
-        "${TEMPO_PROFILE_PREFIX}NETWORK=$CURRENT_NETWORK FILE_SUFFIX=$FILE_SUFFIX forge script script/tasks/solidity/AcceptOwnershipTransferPeriphery.s.sol --fork-url $CURRENT_NETWORK --json --broadcast --verify --skip-simulation ${LEGACY_CLI_FLAG}--tc DeployScript" \
+        "${TEMPO_PROFILE_PREFIX}NETWORK=$CURRENT_NETWORK FILE_SUFFIX=$FILE_SUFFIX forge script script/tasks/solidity/AcceptOwnershipTransferPeriphery.s.sol --fork-url $CURRENT_NETWORK --json --broadcast --verify --skip-simulation ${TEMPO_FEE_CLI_FLAG}${LEGACY_CLI_FLAG}--tc DeployScript" \
         "true"
As per coding guidelines, "CRITICAL for command construction: ensure the dynamically injected flags (tempo profile prefix, `--legacy` inclusion/exclusion, fee-token flag) are applied consistently across all branches (`forge script` primary path and `cast` fallback) and do not change governance routing semantics."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/tasks/acceptOwnershipTransferPeriphery.sh` around lines 53 - 64, The
broadcast command built for AcceptOwnershipTransferPeriphery.s.sol omits the
Tempo fee-token flag declared in config/networks.json, so update the command
construction used by executeAndParse (and the corresponding cast fallback) to
append the Tempo fee-token flag alongside TEMPO_PROFILE_PREFIX and
LEGACY_CLI_FLAG; specifically, modify the line that constructs the forge script
invocation inside the loop (the string passed to executeAndParse) to include the
fee-token flag (the same token configured per-network) and ensure any
alternate/fallback command path where executeAndParse falls back to cast also
receives the identical flag so governance routing/fee-token behavior remains
consistent.

Comment on lines +81 to +88
local TEMPO_PROFILE_PREFIX
TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
local LEGACY_CLI_FLAG
LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")

# Execute, parse, and check return code
if ! executeAndParse \
"NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast --legacy $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
"${TEMPO_PROFILE_PREFIX}NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast ${LEGACY_CLI_FLAG}$SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add the Tempo fee-token flag to this broadcast path.

Line 88 only injects the Tempo profile and legacy switch. config/networks.json Line 1284 now declares Tempo’s --tempo.fee-token ..., but updateFacetConfig() does not read customDeployFlags, so this forge script --broadcast path will still submit Tempo transactions without the required fee-token flag.

🔧 Suggested fix
       local TEMPO_PROFILE_PREFIX
       TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
       local LEGACY_CLI_FLAG
       LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
+      local TEMPO_FEE_CLI_FLAG
+      TEMPO_FEE_CLI_FLAG=$(getTempoFeeTokenCliFlag "$NETWORK")
 
       # Execute, parse, and check return code
       if ! executeAndParse \
-        "${TEMPO_PROFILE_PREFIX}NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast ${LEGACY_CLI_FLAG}$SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
+        "${TEMPO_PROFILE_PREFIX}NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast ${TEMPO_FEE_CLI_FLAG}${LEGACY_CLI_FLAG}$SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
         "true" \
         "forge script failed for $SCRIPT on network $NETWORK" \
         "continue"; then
As per coding guidelines, "CRITICAL for command construction: ensure the dynamically injected flags (tempo profile prefix, `--legacy` inclusion/exclusion, fee-token flag) are applied consistently across all branches (`forge script` primary path and `cast` fallback) and do not change governance routing semantics."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
local TEMPO_PROFILE_PREFIX
TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
local LEGACY_CLI_FLAG
LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
# Execute, parse, and check return code
if ! executeAndParse \
"NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast --legacy $SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
"${TEMPO_PROFILE_PREFIX}NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast ${LEGACY_CLI_FLAG}$SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
local TEMPO_PROFILE_PREFIX
TEMPO_PROFILE_PREFIX=$(getTempoForgeProfilePrefix "$NETWORK")
local LEGACY_CLI_FLAG
LEGACY_CLI_FLAG=$(getForgeLegacyCliFlag "$NETWORK")
local TEMPO_FEE_CLI_FLAG
TEMPO_FEE_CLI_FLAG=$(getTempoFeeTokenCliFlag "$NETWORK")
# Execute, parse, and check return code
if ! executeAndParse \
"${TEMPO_PROFILE_PREFIX}NETWORK=$NETWORK FILE_SUFFIX=$FILE_SUFFIX USE_DEF_DIAMOND=$USE_MUTABLE_DIAMOND PRIVATE_KEY=$(getPrivateKey \"$NETWORK\" \"$ENVIRONMENT\") forge script \"$SCRIPT_PATH\" --fork-url \"$NETWORK\" --json --broadcast ${TEMPO_FEE_CLI_FLAG}${LEGACY_CLI_FLAG}$SKIP_SIMULATION_FLAG --gas-estimate-multiplier \"$GAS_ESTIMATE_MULTIPLIER\"" \
🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 88-88: Double quote to prevent globbing and word splitting.

(SC2086)


[info] 88-88: Double quote to prevent globbing and word splitting.

(SC2086)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/tasks/updateFacetConfig.sh` around lines 81 - 88, updateFacetConfig()
constructs the forge/cast broadcast command but omits network-specific custom
flags (Tempo's --tempo.fee-token) because it never reads or injects
customDeployFlags; fix by fetching and appending the network's custom deploy
flags into the constructed command used by executeAndParse (alongside
TEMPO_PROFILE_PREFIX, LEGACY_CLI_FLAG, SKIP_SIMULATION_FLAG,
GAS_ESTIMATE_MULTIPLIER) so the --tempo.fee-token from config/networks.json is
included for the `forge script "$SCRIPT_PATH" --fork-url "$NETWORK" --json
--broadcast ...` path and mirror the same injection into the cast fallback
branch; reference getTempoForgeProfilePrefix, getForgeLegacyCliFlag,
customDeployFlags, updateFacetConfig(), and executeAndParse when making the
change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants