The Wayback Machine - https://web.archive.org/web/20210901204629/https://github.com/ethereum/solidity/issues/11767
Skip to content

/ solidity

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

Jump to bottom

[DOCS] Use calldata whenever that is sufficient in solidity-by-example section #11767

Open
hrkrshnn opened this issue Aug 10, 2021 · 7 comments
Open

[DOCS] Use calldata whenever that is sufficient in solidity-by-example section #11767

hrkrshnn opened this issue Aug 10, 2021 · 7 comments
Labels
difficulty: easy documentation 📖 good first issue
Projects
Solidity

Comments

@hrkrshnn
Copy link
Member

@hrkrshnn hrkrshnn commented Aug 10, 2021 •
edited 

    /// Reveal your blinded bids. You will get a refund for all
    /// correctly blinded invalid bids and for all bids except for
    /// the totally highest.
    function reveal(
        uint[] memory _values,
        bool[] memory _fake,
        bytes32[] memory _secret
    )

Changing memory to calldata for this example is better.

Unless the code explicitly requires the argument to be in memory, and doesn't modify the argument, keeping it in calldata is more efficient gas wise.

There are a couple of other places in the documentation that could use this change.
@hrkrshnn hrkrshnn added this to New issues in Solidity via automation Aug 10, 2021
@v-sreekesh
Copy link
Contributor

@v-sreekesh v-sreekesh commented Aug 10, 2021

Hi @hrkrshnn could you link me to the location of the file i would like to help with this issue

@hrkrshnn
Copy link
Member Author

@hrkrshnn hrkrshnn commented Aug 10, 2021

@v-sreekesh https://docs.soliditylang.org/en/latest/solidity-by-example.html#id2

@v-sreekesh v-sreekesh mentioned this issue Aug 12, 2021
Closed
@v-sreekesh
Copy link
Contributor

@v-sreekesh v-sreekesh commented Aug 12, 2021

Hi @hrkrshnn i have made the changes could you review and let me know thanks

@hrkrshnn
Copy link
Member Author

@hrkrshnn hrkrshnn commented Aug 12, 2021

Other examples

constructor(bytes32[] memory proposalNames)

Functions in contract ReceiverPays and contract SimplePaymentChannel can also be changed. But this requires some knowledge about how data is encoded in calldata, so probably out of scope for a "good first issue"

@v-sreekesh
Copy link
Contributor

@v-sreekesh v-sreekesh commented Aug 12, 2021

Other examples

constructor(bytes32[] memory proposalNames)

Functions in contract ReceiverPays and contract SimplePaymentChannel can also be changed. But this requires some knowledge about how data is encoded in calldata, so probably out of scope for a "good first issue"

Can I refer some resources online for further understanding to fix this issue

@hrkrshnn
Copy link
Member Author

@hrkrshnn hrkrshnn commented Aug 16, 2021 •
edited

@v-sreekesh Maybe https://docs.soliditylang.org/en/v0.8.7/types.html?highlight=calldata#data-location?

I would recommend running the example in remix, and seeing how the gas changes when memory is replaced by `calldata. If you are keen, you can try to run the debugger on and see the differences.

@v-sreekesh
Copy link
Contributor

@v-sreekesh v-sreekesh commented Aug 16, 2021

@v-sreekesh Maybe https://docs.soliditylang.org/en/v0.8.7/types.html?highlight=calldata#data-location?

I would recommend running the example in remix, and seeing how the gas changes when memory is replaced by `calldata. If you are keen, you can try to run the debugger on and see the differences.

Will check and compare the difference in remix for gas consumption

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
difficulty: easy documentation 📖 good first issue
Projects
Solidity
  
New issues
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@hrkrshnn @v-sreekesh