Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Compiler issue when compiling Actor.sol Return type not supported #172

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

NufailIsmath
Copy link
Contributor

@NufailIsmath NufailIsmath commented Jan 25, 2023

Modified the getErrorCodeMsg function.

  • Removed if condition because it returns the same outside the condition too.
  • Used abi.encodePacked() to concat string instead of string.concat (for low gas consumption).

The screenshot of the error:
image

🔗 zboto Link

@CLAassistant
Copy link

CLAassistant commented Jan 25, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ NufailIsmath
✅ emmanuelm41
❌ jleni
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

@fabriziogianni7 fabriziogianni7 left a comment

Choose a reason for hiding this comment

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

changing this and compiling works for me

@BlocksOnAChain
Copy link

BlocksOnAChain commented Jan 25, 2023

@emmanuelm41 what are the next steps for this PR?

@emmanuelm41
Copy link
Member

emmanuelm41 commented Jan 26, 2023

changing this and compiling works for me

With solc version 0.8.17 it compiles correctly too, as you can see in CI. Which compiler version did you use? So we can test it too.

@emmanuelm41
Copy link
Member

emmanuelm41 commented Jan 26, 2023

In order to let you be a collaborator in the repo, I just fix the error here, and let you apply the abi.encodePacked improvement. Please, rebase your branch and update the PR! #175

@NufailIsmath
Copy link
Contributor Author

NufailIsmath commented Jan 26, 2023

Sure

@emmanuelm41
Copy link
Member

emmanuelm41 commented Jan 26, 2023

@NufailIsmath ready to rebase the branch!

@NufailIsmath
Copy link
Contributor Author

NufailIsmath commented Jan 27, 2023

@emmanuelm41 I did the changes. can you have a look at it

@jleni jleni self-requested a review January 27, 2023 13:01
@emmanuelm41 emmanuelm41 self-requested a review January 27, 2023 13:05
@emmanuelm41 emmanuelm41 merged commit c4ccea9 into Zondax:master Jan 31, 2023
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.

7 participants