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

Behavior of memory to storage copies #218

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

shellygr
Copy link
Contributor

No description provided.

@shellygr shellygr marked this pull request as ready for review February 16, 2024 10:35
@urikirsh
Copy link
Contributor

urikirsh commented Sep 4, 2024

Going over the stale open doc PRs. @shellygr is this still relevant? If so, I'll review it

@shellygr
Copy link
Contributor Author

shellygr commented Sep 4, 2024

Should be still true

Copy link
Contributor

@urikirsh urikirsh left a comment

Choose a reason for hiding this comment

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

Small fixes and a question

This is done by the compiler using a 'copy-loop'.
The selector component `this.testPush.selector` requires one iteration.
the string `aa` requires three iterations: according to the [ABI specification](https://docs.soliditylang.org/en/v0.8.24/abi-spec.html),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the string `aa` requires three iterations: according to the [ABI specification](https://docs.soliditylang.org/en/v0.8.24/abi-spec.html),
The string `aa` requires three iterations: according to the [ABI specification](https://docs.soliditylang.org/en/v0.8.24/abi-spec.html),

Yes - when we copy `data` from memory to storage, the Solidity compiler
also generates code that nullifies the previous data.
As we do not know (and probably not wishing to constrain) the size of the previous data,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
As we do not know (and probably not wishing to constrain) the size of the previous data,
As we do not know (and probably do not wish to constrain) the size of the previous data,

```

The only difference in this new functionality is that we assume that the previous data has size of 225 bytes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The only difference in this new functionality is that we assume that the previous data has size of 225 bytes.
The only difference in this new functionality is that we assume that the previous data has a size of 225 bytes.

If we wish to analyze this code with the Prover, there are two questions to be answered:

1. Do we need to set a value for `--loop_iter` which is bigger than 1?
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the prover smart enough to unroll the loop enough times for this case? The string is static

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.

2 participants