Skip to content
This repository has been 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 :)

src/core/resolvers/block.ts Outdated Show resolved Hide resolved
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