Skip to content

Conversation

@chfast
Copy link
Member

@chfast chfast commented May 18, 2018

  1. Fix blockhash contract for block number 1
    This fixes the issue in the contract when executed from SYSTEM account at block 1. Then the number of the previous block is 0 and the contract executes the loop forever.

  2. Correctly handle argument with negative value
    Fix fixes the issue in the blockhash contract where the negative value of the argument is not properly handled.

chfast added 2 commits May 18, 2018 11:30
This fixes the issue in the contract when executed from SYSTEM account at block 1. Then the number of the previous block is 0 and the contract executes the loop forever.
Fix fixes the issue in the blockhash contract where the negative value of the argument is not properly handled.
@eip-automerger
Copy link

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

  • EIP 210 requires approval from one of (@vbuterin)

@chfast
Copy link
Member Author

chfast commented May 18, 2018

I'm not sure I extracted runtime code correctly. @chriseth @axic can you take a look?

@holiman
Copy link
Contributor

holiman commented May 18, 2018

That blocknum 1 issue is actually very important, because otherwise it'll produce very weird results in most test scenarios

@gumb0
Copy link
Member

gumb0 commented May 18, 2018

We still don't have any tests for the contract code itself, do we?

@chfast
Copy link
Member Author

chfast commented May 18, 2018

We do, but for old version of the contract: #641. When migrating to new code I discovered this block 1 bug.

@chfast
Copy link
Member Author

chfast commented May 18, 2018

Because these 2 bugs are quite obvious, I'd like to fix them first before progressing with tests.

@axic
Copy link
Member

axic commented Sep 20, 2018

I'm not sure I extracted runtime code correctly. @chriseth @axic can you take a look?

This seems to be Vyper so I am not sure, but it looks okay.

Can this be merged?

@Arachnid Arachnid merged commit e58c4d5 into ethereum:master Oct 15, 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.

6 participants