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

fix status_code of init, get_deposit_root, and get_deposit_count #316

Merged
merged 3 commits into from Feb 21, 2020

Conversation

daejunpark
Copy link
Contributor

No description provided.

[ 536 := #buf(32, 8) ] /* for slice(convert(y, bytes32), start=24, len=8) */
[ 704 := #buf(32, 8) ++ #bufSeg(#buf(32, Y8), 24, 8) ] /* return: bytes[8] */ /* memcpy(704 <- 536, 40) */
[ 352 /* = 320 + 32 * 1 */ := #buf(32, {RETURN_ADDR_FROM_TO_LITTLE_ENDIAN_64}) ] /* return address */
[ 320 /* = 320 + 32 * 0 */ := #buf(32, DEPOSIT_COUNT) ] /* argument */
Copy link

Choose a reason for hiding this comment

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

any reason why these are out of order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main reason is that K does not yet fully support the AC matching over maps. So, I had to put the map updates in the order they appear in the execution of the bytecode. The advantage of this order is that it clearly shows how the local memory is updated in each step of the bytecode execution.

[ 672 := #buf(32, 32) ] /* ??? */
[ 640 := #buf(32, 0) ] /* ??? */
[ 800 /* = 320 + 32 * 15 */ := #buf(32, 32) ] /* padding loop index */
[ 744 /* = 320 + 32 * 13 + 8 */ <- 0 ] /* padding */
Copy link

Choose a reason for hiding this comment

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

[ 744 /* = 320 + 32 * 13 + 8 */ := #buf(22, 0) ] would rewrite to this, no? That seems cleaner 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.

Yes, they are equivalent, but the current form is easier for K to prove. Your succinct form requires more lemmas for K to prove.

@MrChico
Copy link

MrChico commented Feb 21, 2020

I noticed you assume memUsed to be 0 in the rhs of to_little_endian_64 spec here. This should probably be abstracted to some MU instead, since this is an internal function

@daejunpark
Copy link
Contributor Author

@MrChico You're right. But note that since no MSIZE opcode appears in the bytecode, the only effect of having it to be 0 in the LHS is that the GAS_COST value of the specification is an over-approximation of the actual cost. The precise (symbolic) expression of the gas cost for the entire contract was not in the scope.

I noticed you assume memUsed to be 0 in the rhs of to_little_endian_64 spec here. This should probably be abstracted to some MU instead, since this is an internal function

@daejunpark
Copy link
Contributor Author

@MrChico I'll merge this PR for now, as your questions are not necessarily relevant to the changes in this PR. Please open github issues if your questions are not fully addressed by my answers, so that we can discuss further. Thanks for your detailed review!

@daejunpark daejunpark merged commit ac4f0bb into master Feb 21, 2020
@daejunpark daejunpark deleted the deposit branch February 21, 2020 20:36
@daejunpark daejunpark changed the title more comments on memory addresses fix status_code of init, get_deposit_root, and get_deposit_count Feb 21, 2020
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