Skip to content
This repository was archived by the owner on Mar 5, 2024. It is now read-only.

Conversation

vihu
Copy link
Contributor

@vihu vihu commented May 13, 2022

Problem

When ledger POC verification fails we can't really tell whether it failed due to incorrect challenger or the block hash.

Solution

This switches the return type for poc_v3:verify to be an ok | {error, ...} and passes it down.

@vihu vihu force-pushed the rg/poc-verification-specific-error branch from 0d43362 to 684c189 Compare May 13, 2022 17:01
case blockchain_ledger_poc_v3:verify(PoC, Challenger, BlockHash) of
false -> {error, mismatched_poc};
true ->
{error, R} -> {error, {mismatched_poc, R}};
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to update the spec of verify_poc_details/3 with this new error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 12860ea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did it slightly differently though and made the error return a tuple of tuples.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sensible. So close to a record!

@vihu vihu force-pushed the rg/poc-verification-specific-error branch from 9569a43 to 12860ea Compare May 18, 2022 17:15
case blockchain_ledger_poc_v3:verify(PoC, Challenger, BlockHash) of
false -> {error, mismatched_poc};
true ->
{error, R} -> {error, {mismatched_poc, R}};
Copy link
Contributor

Choose a reason for hiding this comment

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

Sensible. So close to a record!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants