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

state_transition should use SignedBeaconBlock #762

Closed
mratsim opened this issue Feb 21, 2020 · 9 comments
Closed

state_transition should use SignedBeaconBlock #762

mratsim opened this issue Feb 21, 2020 · 9 comments
Labels
good first issue Good for newcomers

Comments

@mratsim
Copy link
Contributor

mratsim commented Feb 21, 2020

Missed interface change in #647

Normally an easy change (don't forget to update nbench #747 and the blockpool)

https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/beacon-chain.md#beacon-chain-state-transition-function

@mratsim mratsim added the good first issue Good for newcomers label Feb 21, 2020
@JGcarv
Copy link
Contributor

JGcarv commented Feb 27, 2020

Hello @mratsim

Is anyone working on this yet? Can I tackle it?

@arnetheduck
Copy link
Member

All yours! We work off the devel branch for PR's. Let us know if you start working on it.

@JGcarv
Copy link
Contributor

JGcarv commented Feb 28, 2020

Good! I'll start later today. Thank you

@arnetheduck
Copy link
Member

the PR highlights an interesting issue: the invalid_block_sig test is detecting a randao mismatch, so it doesn't detect that we're not validating the block signature - it passes though it shouldn't - this is a problem, because it meant that our test suite did not detect that we were not processing the block correctly - it needs further investigation.

https://github.com/ethereum/eth2.0-spec-tests/tree/master/tests/minimal/phase0/sanity/blocks/pyspec_tests/invalid_block_sig

@JGcarv
Copy link
Contributor

JGcarv commented Mar 8, 2020

I submitted a new PR finish the remaining work on this ticket.

Also, regarding the issue with invalid_block_sig, my guess is because randao is evaluated before the block sig on the sanity tests, and it was outdated causing it to always fail.

But it would be good to double check once state_transition is completed and #780 is merged!

@arnetheduck
Copy link
Member

Also, regarding the issue with invalid_block_sig, my guess is because randao is evaluated before the block sig on the sanity tests, and it was outdated causing it to always fail.

regarding this though, there are two possibilities: the randao is correct and if it wasn't for the signature, the test would pass - or the randao is broken, but nobody noticed because they have their sig check in order. I'd be curious to know which it is - if it's the latter with broken randao, it's not a good signature test really and we need to report it upstream.

@arnetheduck
Copy link
Member

it should be possible to test this using https://github.com/protolambda/zcli

@tersec
Copy link
Contributor

tersec commented Mar 8, 2020

it should be possible to test this using https://github.com/protolambda/zcli

If the last commit really is v0.9.4 spec zrnt, then not yet. https://github.com/protolambda/zrnt appears to be on 0.9.4 as well.

@arnetheduck
Copy link
Member

obsolete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants