Skip to content

Conversation

@wamimi
Copy link
Contributor

@wamimi wamimi commented Oct 20, 2025

What changed? Why?

This PR fixes two critical issues in the Deploy on base tutorial that were discovered while following the tutorial step-by-step.

Issue 1: Missing --broadcast flag in deployment command

The current documentation provides a deployment command that only performs a dry run simulation without actually deploying the contract to the network. Delopers see transaction details but receive a warning "Dry run enabled, not broadcasting transaction", and no contract address is generated. This causes confusion because users expect successful deployment but no transaction hash is printed to the console.

Changes:

  • Added optional dry run step to explain simulation behavior
  • Updated deployment command to include the --broadcast flag
  • Added tip callout explaining that --broadcast is required for actual deployment
  • Enhanced output format display to show exactly what users will see after deployment

Issue 2: Unclear environment variable setup for verification

The verification section instructs users to run a cast call using $COUNTER_CONTRACT_ADDRESS, but the documentation never clearly explains when to copy the contract address, what the deployment output looks like, or that source .env must be run after adding the variable. This results in an undefined environment variable and a confusing error: error: invalid value 'number()(uint256)' for '[TO]': odd number of digits.

Changes:

  • Added explicit step showing the deployment output format with clear labels
  • Clarified instructions to copy the "Deployed to:" address to .env
  • Added reminder to run source .env after modifying the file
  • Included tip in verification section warning about undefined environment variables

These changes prevent silent failures, make environment variable setup explicit, and provide helpful guidance at points where developers are likely to get stuck.

Notes to reviewers

The changes maintain the tutorial's flow while adding critical missing information. The new tips use the existing <Tip> component for consistency with the rest of the documentation. All instructions are actionable and specific, reducing ambiguity for first-time users.

How has it been tested?

I followed the updated tutorial from start to finish on Base Sepolia and verified that:

  • The dry run command shows simulation without deploying
  • The deployment command with --broadcast successfully deploys the contract
  • The contract address and transaction hash appears in the deployment output as documented
  • Adding the address to .env and running source .env works correctly
  • The verification command successfully calls the contract
  • The transaction appears on Sepolia Basescan

I also spun up the documentation site locally and reviewed the updated pages to ensure the formatting displays correctly, the tips render properly, and the content flows naturally within the existing tutorial structure.

Add missing --broadcast flag to forge create command and clarify
COUNTER_CONTRACT_ADDRESS environment variable setup steps to prevent
deployment and verification failures.
@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Oct 20, 2025

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@wamimi
Copy link
Contributor Author

wamimi commented Oct 20, 2025

@youssefea

Copy link

@Orze-yac Orze-yac left a comment

Choose a reason for hiding this comment

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

build the future

@youssefea youssefea merged commit fbe5a3a into base:master Oct 20, 2025
2 checks passed
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.

4 participants