Skip to content
This repository was archived by the owner on Jan 21, 2022. It is now read-only.

Block->ommers shows ommers of a given block. #104

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

akhila-raju
Copy link
Contributor

Resolves #101.

@akhila-raju akhila-raju self-assigned this Aug 8, 2018
@akhila-raju akhila-raju requested a review from raulk August 8, 2018 20:42
@akhila-raju akhila-raju changed the title Shows ommers of a given block. Block->ommers shows ommers of a given block. Aug 8, 2018
@@ -117,6 +117,23 @@ export class EthqlBlock implements EthqlBlock {
}
}

export interface EthqlOmmerBlock
Copy link
Contributor

@raulk raulk Aug 9, 2018

Choose a reason for hiding this comment

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

@akhila-raju – maybe you could elaborate on why it was necessary to introduce a separate class for ommer blocks?

Also why overwrite the miner property in this PR and not in #102?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - the reasons why I introduced a separate class for ommers is because ommers don't contain transactions:

  • transactions shouldn't be a property on ommers if they don't exist
  • transactionsCount will always return null for ommers

Rather than carry down a check for whether the block is an ommer block, I decided to create a new class to simplify things.

The miner property was already EthqlAccount, it wasn't overwritten - just copied and pasted for EthqlOmmerBlock :)

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

Successfully merging this pull request may close these issues.

2 participants