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

Improved storage variable layout #793

Merged
merged 4 commits into from
May 7, 2018

Conversation

jacqueswww
Copy link
Contributor

- What I did

Fixes #769

- How I did it

- How to verify it

- Description for the changelog

- Cute Animal Picture

'MSTORE',
'PUSH1', 64, 'PUSH1', MemoryPositions.FREE_VAR_SPACE, 'SHA3'
])
return o
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daejunpark would be great if you could double check this for me 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

sha3_64 looks good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great thanks ;)

FREE_VAR_SPACE2 = 224
BLANK_SPACE = 256
FREE_LOOP_INDEX = 288
RESERVED_MEMORY = 320
Copy link
Contributor

Choose a reason for hiding this comment

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

@jacqueswww Is it intended to have the same size of the reserved memory even if we have the additional one (FREE_VAR_SPACE2)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daejunpark there seemed to have been an extra slot btween 288 and 320 that was never used. So I took the liberty of just shifting the addresses up.

@daejunpark
Copy link
Contributor

daejunpark commented May 1, 2018

@jacqueswww Sorry for the delay, but I'd like to take a closer look at parser_utils.py sometimes later this week (I'm traveling now), if it is not urgent to merge.

In the meantime, @yzhang90 could you take a look at this as well?

return LLLnode.from_list(['sha3_64', parent, sub],
typ=subtype,
location='storage')
elif location == 'storage_prehashed':
Copy link

Choose a reason for hiding this comment

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

location = storage and location = storage_prehashed use different hash-schema? when the location will be storage_hashed?

Copy link

Choose a reason for hiding this comment

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

Please confirm that storage_prehashed is only used in List, Struct and Tuple. You don't need to consider it when the type is Map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that is correct, prehashed should only occur with != Map I will remove this section ;)

@jacqueswww jacqueswww merged commit 0d92d86 into vyperlang:master May 7, 2018
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.

None yet

3 participants