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

VerifyConsensusFault syscall impl #1534

Merged
merged 4 commits into from
Apr 16, 2020
Merged

VerifyConsensusFault syscall impl #1534

merged 4 commits into from
Apr 16, 2020

Conversation

sternhenri
Copy link
Contributor

@sternhenri sternhenri commented Apr 10, 2020

This impl is heavily based on the structure of https://github.com/filecoin-project/go-filecoin/blob/d75e700d2b5b116a794c3da672d6379bcb24b803/internal/pkg/slashing/check.go#L40.

A few things to note:

  • No check whether blocks are in chain yet. I am not sure they are needed.
  • Implementing tipset operations (e.g. Contains) based on CIDs rather than tipsets given the headers submitted to VerifyConsensusFault may be invalid (and yet the faults themselves may not be).
  • Should maybe use VerifySignature instead of making VerifyBlockSig

@sternhenri sternhenri changed the base branch from master to testnet/3 April 10, 2020 15:18
@sternhenri sternhenri marked this pull request as draft April 10, 2020 15:21
@sternhenri sternhenri changed the title [WIP] VerifyConsensusFault syscall impl VerifyConsensusFault syscall impl Apr 10, 2020
@whyrusleeping whyrusleeping marked this pull request as ready for review April 16, 2020 19:06
return nil, xerrors.Errorf("cannot decode extra: %w", decodeErr)
}

if types.CidArrsEqual(blockA.Parents, blockC.Parents) && blockA.Height == blockB.Height &&
Copy link
Contributor

@Kubuxu Kubuxu Apr 16, 2020

Choose a reason for hiding this comment

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

from what I'm reading, a.Height and b.Height will be always different in this case, code says something else.

also with that check it would be a more restricted case (a)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah typo, it is blockA.Height == blockC.Height. Great catch.

Do you see how it's different from a?

Copy link
Contributor

@Kubuxu Kubuxu Apr 20, 2020

Choose a reason for hiding this comment

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

yeah, now it makes sense.

@whyrusleeping whyrusleeping merged commit d56b5e2 into testnet/3 Apr 16, 2020
@whyrusleeping whyrusleeping deleted the patch/syscalls branch April 16, 2020 20:34
Kubuxu pushed a commit that referenced this pull request May 12, 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