Skip to content

Conversation

@olegrok
Copy link
Contributor

@olegrok olegrok commented Jun 30, 2025

This patch removes external deployment from public interfaces (CLI, niljs) and replaces it with deployment via faucet.
We need it to support EOA because each address with positive balance and without contract code should be considered as EOA. But right now this address can be used to deploy a contract.

@olegrok olegrok requested a review from Copilot July 1, 2025 06:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the legacy “external deployment” flow and switches all public interfaces to use a faucet-based deployment.

  • Changed selfDeploy signature to accept a FaucetClient instead of a boolean flag.
  • Updated smart account and faucet contracts/tests to use the new deploy RPC.
  • Cleared out deprecated external‐deploy code paths from CLI, examples, docs, and services.

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
wallet-extension/src/features/blockchain/smartAccount.ts Replaced boolean‐flag deploy with faucetClient call
smart-contracts/contracts/Faucet.sol Renamed createSmartAccountdeploy, using asyncDeploy
rollup-bridge-contracts/task/nil-smart-account.ts Drop top-up; always deploy via faucetClient
niljs/src/utils/smart-account.ts Instantiate faucetClient; call selfDeploy(faucetClient)
niljs/src/utils/faucet.ts Added wrapper deploy helper
niljs/src/smart-accounts/SmartAccountV1/SmartAccountV1.ts Updated selfDeploy implementation & docs
niljs/examples/deploySmartAccount.ts Switched example to faucet-based deploy
nil/tests/faucet/faucet_test.go Updated test helper to call new deploy method
nil/tests/cli/cli_test.go CLI tests now use Deploy(...); adjusted seqno
nil/services/**/* Removed old external‐deploy paths; replaced with Deploy
hardhat-plugin/src/{types,index,core/wallet}.ts Exposed faucet client in Hardhat environment
docs/**/* Stripped external deployment docs and tests
Comments suppressed due to low confidence (2)

niljs/examples/deploySmartAccount.ts:19

  • There's a typo in the variable name faucetClinet; consider renaming to faucetClient so it matches the class and other references.
const faucetClinet = new FaucetClient({

niljs/src/smart-accounts/SmartAccountV1/SmartAccountV1.ts:175

  • The docblock parameter is outdated; selfDeploy now takes a FaucetClient instance (and an optional amount), not a string endpoint. Please update the @param and example accordingly.
   * @param {string} [faucetEndpoint] Faucet endpoint.

maxFeePerGas: maxFeePerGas,
return faucetClient.deploy({
shardId: this.shardId,
code: data.slice(0, data.length - 32), // remove salt from code
Copy link

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

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

The hardcoded 32 here is a magic number. Consider extracting it into a named constant or utility (e.g., SALT_LENGTH) to clarify why that slice is needed.

Suggested change
code: data.slice(0, data.length - 32), // remove salt from code
code: data.slice(0, data.length - SALT_LENGTH), // remove salt from code

Copilot uses AI. Check for mistakes.
This patch removes external deployment from public interfaces (CLI, niljs)
and replaces it with deployment via faucet.
We need it to support EOA because each address with positive balance and without
contract code should be considered as EOA. But right now this address can be used
to deploy a contract.
@joedy27
Copy link

joedy27 commented Jul 6, 2025

Pull Request Overview

This PR removes the legacy “external deployment” flow and switches all public interfaces to use a faucet-based deployment.

  • Changed selfDeploy signature to accept a FaucetClient instead of a boolean flag.
  • Updated smart account and faucet contracts/tests to use the new deploy RPC.
  • Cleared out deprecated external‐deploy code paths from CLI, examples, docs, and services.

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file
Comments suppressed due to low confidence (2)

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