Skip to content

Make testing guard upgrades more ergonomic - #98

Open
makemake-kbo wants to merge 4 commits into
masterfrom
fix/make-guard-upgrade-testing-ergonomic
Open

Make testing guard upgrades more ergonomic#98
makemake-kbo wants to merge 4 commits into
masterfrom
fix/make-guard-upgrade-testing-ergonomic

Conversation

@makemake-kbo

Copy link
Copy Markdown
Contributor

Summary

  • generalize the live-Anvil credible-block integration runner to test a caller-specified upgraded contract and registry configuration
  • verify guard configuration, expected revert behavior, strict fail-open boundaries, and caller-provided state effects
  • keep GuardedCounter as a clearly labeled default fixture and document real-upgrade usage
  • add argument parsing and failure-message regression tests

Validation

  • ./examples/credible-block/test/test-script-arguments.sh — 7 passed
  • default live-Anvil fixture run — 14 passed, 0 failed
  • configurable deployment-command live-Anvil run — 14 passed, 0 failed
  • bash -n and git diff --check — passed

Comment thread examples/credible-block/script/test-credible-upgrades.sh Outdated
Comment thread examples/credible-block/script/test-credible-upgrades.sh Outdated
Comment thread examples/credible-block/script/test-credible-upgrades.sh
Comment thread examples/credible-block/script/test-credible-upgrades.sh
Comment thread examples/credible-block/script/test-credible-upgrades.sh Outdated
mateo-mro
mateo-mro previously approved these changes Aug 3, 2026
Comment thread examples/credible-block/script/test-credible-upgrades.sh

@lean-apple lean-apple left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MARKER_ADDRESS=$(cast wallet address "$MARKER_KEY")
GUARDED_ADDRESS=$(cast wallet address "$GUARDED_KEY")
export RPC_URL REPO_ROOT ADMIN_KEY MARKER_KEY GUARDED_KEY MARKER_ADDRESS GUARDED_ADDRESS
export EXPECTED_THRESHOLD="$FAIL_OPEN_THRESHOLD"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Export the target before the registry command

TARGET_ADDRESS has been parsed here but is not exported before the registry deployment/configuration command runs. Consequently, --target-address … --registry-deploy-command '…$TARGET_ADDRESS…' sees an empty variable even though the README and help text promise it to deployment commands. Please normalize and export a supplied target address before executing the first user command, and add an environment-passing regression case.

receipt_block() { cast to-dec "$(cast receipt --rpc-url "$RPC_URL" --async "$1" --json | jq -r '.blockNumber')"; }
normalize_cast_value() {
local value="$1"
printf '%s' "${value%% \[*}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Anchor numeric annotation normalization

This removes everything from the first [ anywhere in the value. A valid ABI string such as "hello [world]" is therefore normalized to "hello, breaking the newly supported string-state comparison. Please strip the annotation only when the complete output matches cast’s numeric annotated form and otherwise preserve the value unchanged. A live bracket-containing string case would cover this.

--registry-address) need_value "$@"; REGISTRY_ADDRESS="$2"; shift 2 ;;
--registry-deploy-command) need_value "$@"; REGISTRY_DEPLOY_COMMAND="$2"; shift 2 ;;
--guarded-call) need_value "$@"; GUARDED_CALL="$2"; shift 2 ;;
--guarded-call-arg) need_value "$@"; GUARDED_CALL_ARGS+=("$2"); shift 2 ;;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Permit empty ABI arguments

The shared need_value check requires -n "$2", so --guarded-call-arg "", --marker-call-arg "", and --state-read-call-arg "" are rejected even though an empty string is a valid ABI argument. Please distinguish a missing option operand from a present-but-empty call argument, and add an empty-string target-mode regression.

receipt_block() { cast to-dec "$(cast receipt --rpc-url "$RPC_URL" --async "$1" --json | jq -r '.blockNumber')"; }
normalize_cast_value() {
local value="$1"
printf '%s' "${value%% \[*}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This normalization runs for every ABI return, so a valid string such as "hello [world]" is truncated at the bracket even though the annotation stripping is only needed for numeric output. Could we restrict this to integer return signatures and keep string values untouched, with a bracket bearing string regression?

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.

3 participants