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

Added block add event. #637

Merged
merged 5 commits into from
Apr 2, 2020
Merged

Added block add event. #637

merged 5 commits into from
Apr 2, 2020

Conversation

davemec
Copy link
Contributor

@davemec davemec commented Apr 1, 2020

PR description

Added block add event.
Added block reorg event.
Added revert reason to block add event.

Signed-off-by: David Mechler david.mechler@consensys.net

Fixed Issue(s)

Added block reorg event.
Added revert reason to both events.

Signed-off-by: David Mechler <david.mechler@consensys.net>
Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

LGTM once the plugin issue is addressed.

(event, chain) ->
listener.onBlockAdded(
blockAddedContext(
event.getBlock()::getHeader,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow... that compiled? I didn't know that was possible.


/** A transaction receipt, containing information pertaining a transaction execution. */
public interface TransactionReceipt {
Optional<Bytes> getRevertReason();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add the 4 fields listed in section 4.3.1 of the yellow paper: cumulativeGasUsed, logs, bloomFilter, and status. For bloomFilter I think we want to just expose the Bytes object rather than our LogBloomFilter. If we get covariant type complaints from Java we can just create a new method (BloomFilterBytes) and cast it down., but for the other three we can just expose the existing methods, since we have Log already as a data type.

Also, they all the methods need javadoc. RevertReason is our own field so we should mention it's data beyond what the yellow paper requires, but for the other four just stick with the simple definitions.

@shemnon shemnon merged commit 061e24a into hyperledger:master Apr 2, 2020
@davemec davemec deleted the PIE-2242_2 branch April 2, 2020 18:04
shemnon pushed a commit that referenced this pull request Apr 7, 2020
Signed-off-by: David Mechler <david.mechler@consensys.net>
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.

2 participants