Skip to content

fix(relay-vaults): externalize hardcoded block explorer API credentials in Hardhat config - #15

Open
mertcano wants to merge 4 commits into
relayprotocol:mainfrom
mertcano:mertcano-patch-1
Open

mertcano wants to merge 4 commits into
relayprotocol:mainfrom
mertcano:mertcano-patch-1

Conversation

@mertcano

Copy link
Copy Markdown

Motivation

This PR hardens the relay-vaults smart contract configuration (smart-contracts/hardhat.config.ts), addressing a high-severity security finding from the workspace security audit[cite: 24, 25]. Previously, multiple block-explorer API credentials were embedded directly as source literals, exposing reusable secrets and requiring a code change for every rotation[cite: 24, 25].

Modifications

  • Environment-Backed Explorer Configuration (smart-contracts/hardhat.config.ts):
    • Introduced the explorerApiKey() helper to dynamically source credentials from network-specific EXPLORER_API_KEY_<NETWORK> environment variables, falling back safely to a placeholder value when unconfigured[cite: 24, 25].
  • Comment Normalization (smart-contracts/hardhat.config.ts):
    • Corrected the touched source comment from // Interracting to standard English (// Interaction tasks)[cite: 24, 25].

Checklist

  • Format your code according to the Contributor Guide.
  • Add unit tests as outlined in the Contributor Guide.
  • Update documentation as needed, including docstrings or example tutorials.

…ls in Hardhat config

## Motivation

This PR hardens the `relay-vaults` smart contract configuration (`smart-contracts/hardhat.config.ts`), addressing a high-severity security finding from the workspace security audit[cite: 24, 25]. Previously, multiple block-explorer API credentials were embedded directly as source literals, exposing reusable secrets and requiring a code change for every rotation[cite: 24, 25].

## Modifications

* **Environment-Backed Explorer Configuration (`smart-contracts/hardhat.config.ts`)**:
  * Introduced the `explorerApiKey()` helper to dynamically source credentials from network-specific `EXPLORER_API_KEY_<NETWORK>` environment variables, falling back safely to a placeholder value when unconfigured[cite: 24, 25].
* **Comment Normalization (`smart-contracts/hardhat.config.ts`)**:
  * Corrected the touched source comment from `// Interracting` to standard English (`// Interaction tasks`)[cite: 24, 25].

## Checklist

- [x] Format your code according to the Contributor Guide.
- [ ] Add unit tests as outlined in the Contributor Guide.
- [x] Update documentation as needed, including docstrings or example tutorials.
```[cite: 24, 25]
@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

This change moves block-explorer verification credentials to network-specific environment variables and documents the required configuration convention.

Confidence Score: 5/5

The credential configuration change is safe to merge.

No blocking failure remains.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex ran the readme fence validation with node trex-artifacts/validate-readme-fence.mjs from /home/user/repo and observed an opening bash fence at line 30 with no closing fence, while lines 35 and 39 remain; the marked renderer produced a single code block containing both the explorer note and Deployment Sequence, and Deployment Sequence did not render as a normal heading.

T-Rex Ran code and verified through T-Rex

Reviews (3): Last reviewed commit: "Enhance Etherscan API key validation in ..." | Re-trigger Greptile

Comment on lines +93 to +95
const explorerApiKey = (network: string): string =>
process.env[`EXPLORER_API_KEY_${network.toUpperCase().replace(/[^A-Z0-9]+/g, '_')}`] ??
'default-api-key'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Explorer key names are undocumented

explorerApiKey requires exact normalized variables such as EXPLORER_API_KEY_ARBITRUM_SEPOLIA and EXPLORER_API_KEY_POLYGON_ZK_EVM, but the deployment documentation does not describe that convention or provide examples. Plausible alternative names silently fall back to default-api-key, and the verification task treats that non-empty placeholder as configured. Document the naming rule and representative network names alongside the verification instructions so operators do not learn about the error only after an explorer-side verification failure.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Focused Hardhat configuration validation script source

  • The uploaded diff contains the exact JavaScript harness executed through Hardhat to read both configured explorer API keys and assert each expected result.

Hardhat configuration with plausible alternate API-key names

  • A real Hardhat run with unnormalized alternate variable names resolved Arbitrum Sepolia and Polygon zkEVM to `default-api-key`, demonstrating the silent fallback.

Hardhat configuration with exact normalized API-key names

  • A real Hardhat run with the exact normalized names resolved both explorer API keys to their supplied values, confirming the required names.

Operator documentation search for explorer API-key names

  • The executed search found the relevant normalized names only in `hardhat.config.ts` and none in the repository documentation, confirming the documentation gap.

View artifacts

T-Rex Ran code and verified through T-Rex

Updated README to include block explorer API keys for contract verification and modified deployment instructions.
Added detailed deployment instructions and usage guide for the Relay Protocol, including steps for deploying contracts, adding liquidity, and bridging tokens.
Added checks for valid Etherscan API key and improved error messages.
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