Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impersonation in E2E #704

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Impersonation in E2E #704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
636f0c8
fix: default to SALT from env
danielattilasimon Jan 17, 2025
a9fedfb
feat: ability to impersonate a deployer
danielattilasimon Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions contracts/fork
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ init_env() {
[ -n "$FORK_BLOCK_NUMBER" ] || FORK_BLOCK_NUMBER=$(cast block-number --rpc-url "$FORK_URL")
[ -n "$FORK_CHAIN_ID" ] || FORK_CHAIN_ID=$(cast chain-id --rpc-url "$FORK_URL")
[ -n "$DEPLOYER" ] || DEPLOYER=$anvil_account_0

if [ ${#DEPLOYER} = 42 ]; then
impersonating=yes
fi
}

ensure_not_running() {
Expand All @@ -59,6 +63,10 @@ start() {
--chain-id "$FORK_CHAIN_ID"
)

if [ "$impersonating" = yes ]; then
anvil+=( --auto-impersonate )
fi

nohup "${anvil[@]}" &> "$logfile" &
echo $! > "$pidfile"
}
Expand Down Expand Up @@ -95,6 +103,10 @@ deploy() {
# --verifier-url "http://localhost:$sourcify_port"
)

if [ "$impersonating" = yes ]; then
deploy+=( --unlocked )
fi

# sourcify_start
"${deploy[@]}" "$@"
# sourcify_stop
Expand Down
62 changes: 35 additions & 27 deletions contracts/utils/deploy-cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { $, echo, fs, minimist } from "zx";
import { $, chalk, echo, fs, minimist } from "zx";

const HELP = `
deploy - deploy the Liquity contracts.
Expand Down Expand Up @@ -29,15 +29,19 @@ Options:
--gas-price <GAS_PRICE> Max fee per gas to use in transactions.
--help, -h Show this help message.
--mode <DEPLOYMENT_MODE> Deploy in one of the following modes:
- complete (default)
- bold-only
- use-existing-bold
--salt SALT used for CREATE2
- complete (default),
- bold-only,
- use-existing-bold.
--open-demo-troves Open demo troves after deployment (local
only).
--rpc-url <RPC_URL> RPC URL to use.
--salt <SALT> Use keccak256(bytes(SALT)) as CREATE2
salt instead of block timestamp.
--slow Only send a transaction after the previous
one has been confirmed.
--unlocked Used when the deployer account is unlocked
in the client (i.e. no private key or
Ledger device needed).
--use-testnet-pricefeeds Use testnet PriceFeeds instead of real
oracles when deploying to mainnet.
--verify Verify contracts after deployment.
Expand All @@ -63,6 +67,7 @@ const argv = minimist(process.argv.slice(2), {
"verify",
"dry-run",
"slow",
"unlocked",
"use-testnet-pricefeeds",
],
string: [
Expand Down Expand Up @@ -171,12 +176,16 @@ export async function main() {
}
}

// Ledger signing
if (options.deployer.startsWith("0x") && options.deployer.length === 42) {
forgeArgs.push("--ledger");
if (options.ledgerPath) {
forgeArgs.push("--hd-paths");
forgeArgs.push(options.ledgerPath);
if (options.unlocked) {
forgeArgs.push("--unlocked");
} else {
// Ledger signing
forgeArgs.push("--ledger");
if (options.ledgerPath) {
forgeArgs.push("--hd-paths");
forgeArgs.push(options.ledgerPath);
}
}
}

Expand All @@ -186,7 +195,7 @@ Deploying Liquity contracts with the following settings:
CHAIN_ID: ${options.chainId}
DEPLOYER: ${options.deployer}
DEPLOYMENT_MODE: ${options.mode}
SALT: ${options.salt ? options.salt : "\u26A0 block.timestamp will be used !!"}
SALT: ${options.salt ? options.salt : chalk.yellow("block.timestamp will be used !!")}
ETHERSCAN_API_KEY: ${options.etherscanApiKey && "(secret)"}
LEDGER_PATH: ${options.ledgerPath}
OPEN_DEMO_TROVES: ${options.openDemoTroves ? "yes" : "no"}
Expand All @@ -199,10 +208,7 @@ Deploying Liquity contracts with the following settings:

process.env.DEPLOYER = options.deployer;
process.env.DEPLOYMENT_MODE = options.mode;

if (options.salt) {
process.env.SALT = options.salt;
}
process.env.SALT = options.salt;

if (options.openDemoTroves) {
process.env.OPEN_DEMO_TROVES = "true";
Expand Down Expand Up @@ -309,6 +315,13 @@ function safeParseInt(value: string) {
return isNaN(parsed) ? undefined : parsed;
}

function envBool(name: string): boolean {
return process.env[name] !== undefined
&& process.env[name].length > 0
&& process.env[name] !== "false"
&& process.env[name] !== "no";
}

async function parseArgs() {
const options = {
chainId: safeParseInt(argv["chain-id"]),
Expand All @@ -323,6 +336,7 @@ async function parseArgs() {
rpcUrl: argv["rpc-url"],
dryRun: argv["dry-run"],
slow: argv["slow"],
unlocked: argv["unlocked"],
verify: argv["verify"],
verifier: argv["verifier"],
verifierUrl: argv["verifier-url"],
Expand All @@ -333,23 +347,17 @@ async function parseArgs() {
const [networkPreset] = argv._;

options.chainId ??= safeParseInt(process.env.CHAIN_ID ?? "");
options.debug ??= Boolean(
process.env.DEBUG && process.env.DEBUG !== "false",
);
options.debug ||= envBool("DEBUG");
options.deployer ??= process.env.DEPLOYER;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I’m missing something, but this seems like a weird behaviour to me:

  • If env var DEBUG is not set, then argument --debug will be used -> fine
  • If env var DEBUG is set to “false” or “no”, then argument --debug will be used -> ??
  • If env var DEBUG is set to “true” or “yes”, then env var will be used -> ??

It seem inconsistent to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see your point, but it's not so easy. The previous behavior was completely broken, because ??= only has an effect on nullish values, but the way we've configured minimist, it returns false when a bool argument such as --debug is missing. Thus, it was impossible to enable debugging via the environment. In that sense, this is already an improvement.

The problem is this:

Note: options can also be set via corresponding environment variables,
e.g. --chain-id can be set via CHAIN_ID instead. Parameters take precedence over variables.

"Parameters take precedence" over variables, but a parameter declared boolean in minimist options is set to false by being omitted. So currently, there's no way to tell the difference between no --debug option being passed or explicitly passing e.g. --debug=false.

I guess we could remove the list of boolean options from the minimist config altogether, and do our own parsing...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#707

options.etherscanApiKey ??= process.env.ETHERSCAN_API_KEY;
options.ledgerPath ??= process.env.LEDGER_PATH;
options.mode ??= process.env.DEPLOYMENT_MODE;
options.openDemoTroves ??= Boolean(
process.env.OPEN_DEMO_TROVES && process.env.OPEN_DEMO_TROVES !== "false",
);
options.openDemoTroves ||= envBool("OPEN_DEMO_TROVES");
options.rpcUrl ??= process.env.RPC_URL;
options.useTestnetPricefeeds ??= Boolean(
process.env.USE_TESTNET_PRICEFEEDS && process.env.USE_TESTNET_PRICEFEEDS !== "false",
);
options.verify ??= Boolean(
process.env.VERIFY && process.env.VERIFY !== "false",
);
options.salt ??= process.env.SALT;
options.unlocked ||= envBool("UNLOCKED");
options.useTestnetPricefeeds ||= envBool("USE_TESTNET_PRICEFEEDS");
options.verify ||= envBool("VERIFY");
options.verifier ??= process.env.VERIFIER;
options.verifierUrl ??= process.env.VERIFIER_URL;

Expand Down
Loading