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.

Sign up for GitHub

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

Jump to bottom

Unnecessary code #2765

Closed
andrijamitrovic23 opened this issue Oct 25, 2023 · 1 comment · Fixed by #2799
Closed

Unnecessary code #2765

andrijamitrovic23 opened this issue Oct 25, 2023 · 1 comment · Fixed by #2799
Assignees
@bao1029p
Labels
audit item is from an audit x/blobstream item is directly relevant to the blob module

Comments

@andrijamitrovic23
Copy link

Involved artifacts

Description
In pruneAttestations function the whole earliest attestation is taken out of the store here using GetEarliestAvailableAttestationNonce. This is done just to get the earliest nonce to use for newEarliestAvailableNonce in the for loop and for updating state and logging here and here. It would be sufficient to get the earliest nonce directly by using GetEarliestAvailableAttestationNonce.

Problem Scenarios
Unnecessary access to the storage.

Recommendation
Just use the functions CheckEarliestAvailableAttestationNonce and GetEarliestAvailableAttestationNonce for getting earliest nonce.
@rach-id rach-id added audit item is from an audit x/blobstream item is directly relevant to the blob module and removed needs:triage labels Oct 25, 2023
@bao1029p
Copy link
Contributor

Hi guys, is this issue open for work ?

@bao1029p bao1029p mentioned this issue Nov 1, 2023
Merged
evan-forbes pushed a commit that referenced this issue Nov 2, 2023
@bao1029p
refactor(x/blobstream): remove unnecessary code (#2799)
e5e98d8 
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Closes: [#2765](#2765)

## Changes

- Call `CheckEarliestAvailableAttestationNonce ` to check if Nonce is
available
- Use `GetEarliestAvailableAttestationNonce ` to fetch the nonce use in
[`newEarliestAvailableNonce`](https://github.com/celestiaorg/celestia-app/blob/2335d2b7d8d9463abc85f79b430926380dd41001/x/qgb/abci.go#L161)
and logging in
[here](https://github.com/celestiaorg/celestia-app/blob/2335d2b7d8d9463abc85f79b430926380dd41001/x/qgb/abci.go#L183)
and
[here](https://github.com/celestiaorg/celestia-app/blob/2335d2b7d8d9463abc85f79b430926380dd41001/x/qgb/abci.go#L189)
0xchainlover pushed a commit to celestia-org/celestia-app that referenced this issue Aug 1, 2024
@bao1029p
refactor(x/blobstream): remove unnecessary code (#2799)
134fa77 
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Closes: [#2765](celestiaorg/celestia-app#2765)

## Changes

- Call `CheckEarliestAvailableAttestationNonce ` to check if Nonce is
available
- Use `GetEarliestAvailableAttestationNonce ` to fetch the nonce use in
[`newEarliestAvailableNonce`](https://github.com/celestiaorg/celestia-app/blob/2335d2b7d8d9463abc85f79b430926380dd41001/x/qgb/abci.go#L161)
and logging in
[here](https://github.com/celestiaorg/celestia-app/blob/2335d2b7d8d9463abc85f79b430926380dd41001/x/qgb/abci.go#L183)
and
[here](https://github.com/celestiaorg/celestia-app/blob/2335d2b7d8d9463abc85f79b430926380dd41001/x/qgb/abci.go#L189)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bao1029p bao1029p

Labels
audit item is from an audit x/blobstream item is directly relevant to the blob module
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(x/blobstream): remove unnecessary code bao1029p/celestia-app
3 participants
@rach-id @andrijamitrovic23 @bao1029p