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.

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

fix: add payable for forceDeploy #71

Merged

Conversation

benceharomi
Copy link
Contributor

@benceharomi benceharomi commented Oct 30, 2023

What ❔

  • updated the IContractDeployer interface: made the create2 and forceDeployOnAddresses payable
  • made the forceDeploy function payable

Why ❔

In the era-system-contracts the ContractDeployer's create2 and forceDeployOnAddresses functions are payable (https://github.com/matter-labs/era-system-contracts/blob/dev/contracts/ContractDeployer.sol), therefore updated the interface and made the forceDeploy function payable.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

Copy link
Collaborator

@StanislavBreadless StanislavBreadless left a comment

Choose a reason for hiding this comment

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

While the function did become payable it makes no use of the msg.value. We should pass the msg.value to the forceDeployOnAddresses call

@benceharomi
Copy link
Contributor Author

While the function did become payable it makes no use of the msg.value. We should pass the msg.value to the forceDeployOnAddresses call

Fixed!

@StanislavBreadless StanislavBreadless changed the base branch from dev to release-v18-boojum October 31, 2023 10:47
@benceharomi benceharomi merged commit f4aa6bb into release-v18-boojum Oct 31, 2023
11 checks passed
@benceharomi benceharomi deleted the bh-evm-345-add-payable-for-forcedeployer branch October 31, 2023 13:54
benceharomi pushed a commit that referenced this pull request Nov 17, 2023
StanislavBreadless added a commit that referenced this pull request Dec 17, 2024
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