Skip to content

fix: Return correct latest block number#545

Merged
igamigo merged 3 commits intonextfrom
igamigo-fix-block-num
Nov 5, 2024
Merged

fix: Return correct latest block number#545
igamigo merged 3 commits intonextfrom
igamigo-fix-block-num

Conversation

@igamigo
Copy link
Collaborator

@igamigo igamigo commented Nov 5, 2024

No description provided.

@igamigo igamigo requested review from bobbinth and polydez November 5, 2024 19:53
@igamigo igamigo added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Nov 5, 2024
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Great catch! Thank you!

/// Returns the latest block number.
fn latest_block_num(&self) -> BlockNumber {
(self.chain_mmr.forest() + 1).try_into().expect("block number overflow")
(self.chain_mmr.forest() - 1).try_into().expect("block number overflow")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would maybe add a safety comment here saying that chain_mmr always has at least one block (the genesis block) - and so this should be fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I thought of the potential underflow but realized it was not an issue but did not write a comment. Will add now.

@igamigo igamigo merged commit abed540 into next Nov 5, 2024
@igamigo igamigo deleted the igamigo-fix-block-num branch November 5, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants