Skip to content

Fix termination condition in zero_pad#1611

Merged
charles-cooper merged 4 commits intovyperlang:masterfrom
charles-cooper:zero_pad
Sep 24, 2019
Merged

Fix termination condition in zero_pad#1611
charles-cooper merged 4 commits intovyperlang:masterfrom
charles-cooper:zero_pad

Conversation

@charles-cooper
Copy link
Copy Markdown
Member

@charles-cooper charles-cooper commented Sep 10, 2019

What I did

Fix #1599, #1610

How I did it

See mentioned issues for discussion

How to verify it

Description for the changelog

Fix zero padding conditions in ABI encoder

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@charles-cooper charles-cooper added the work in progress Work on this PR or issue is not yet complete but reviewers are free to add their input for guidance. label Sep 10, 2019
maxlen was confusingly named. maxlen referred to the static size of the
entire packed log. instead, the maxlen of the current item should be
passed to zero_pad.
bytearrays are padded to ceil32(runtime_len), not ceil32(maxlen).
@charles-cooper charles-cooper changed the title WIP Fix termination condition in zero_pad Fix termination condition in zero_pad Sep 16, 2019
@daejunpark
Copy link
Copy Markdown
Contributor

This fixes the issues that I reported, and will unblock the deposit contract verification.

@djrtwo djrtwo requested a review from davesque September 17, 2019 17:19
@charles-cooper charles-cooper removed the work in progress Work on this PR or issue is not yet complete but reviewers are free to add their input for guidance. label Sep 19, 2019
@jacqueswww
Copy link
Copy Markdown
Contributor

LGTM, @charles-cooper anyway that we could've picked this up in a test?

@charles-cooper
Copy link
Copy Markdown
Member Author

@jacqueswww yeah maybe, I think if there was data in the next cell whose first byte (BE) was nonzero then it would have been corrupted.

@charles-cooper charles-cooper merged commit 32ce6c9 into vyperlang:master Sep 24, 2019
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.

Off-by-one error in zero_pad

3 participants