Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

Some Fixes PlainOpcodes.s.sol #1441

Closed
wants to merge 1 commit into from

Conversation

lordofdegen
Copy link

@lordofdegen lordofdegen commented Sep 24, 2024

Time spent on this PR: 0.5 days

Pull request type: Bugfix

What is the current behavior?

Resolves #1: Handling deployerPrivateKey might throw an error if it's set to 0
If the deployerPrivateKey is null, it could lead to improper handling or unexpected behavior when calling vm.startBroadcast.

Resolves #2: Missing success checks for contract calls
There are no success checks for the Counter and PlainOpcodes contract calls. If the contract deployment fails, it won't be explicitly logged.

Resolves #3: Type issue in the create function
The create function is called with type(Counter).creationCode, which returns the contract bytecode. However, it's unclear if the expected argument for create matches this return value. In the PlainOpcodes contract, create might expect different data types (e.g., hashes or precompiled codes)."

What is the new behavior?

  • Now there's a null check in place.
  • Contract deployments now have success checks.
  • We can now confirm that create expects bytecode, and the passed value is correct. If an address or hash is needed, adjust the parameter.

This change is Reviewable

@enitrat
Copy link
Collaborator

enitrat commented Sep 24, 2024

Hi, I don't think there are any issues with this file

@enitrat enitrat closed this Sep 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants