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

Snowbridge Audit Issue 9 - Unused code should be removed #6100

Closed

Conversation

claravanstaden
Copy link
Contributor

Description

A minor change required by Oak Security audit to remove unused code.

Requires labels R0-silent and T15-bridges.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team October 17, 2024 09:07
@bkchr bkchr added R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges. labels Oct 20, 2024
@@ -131,8 +131,6 @@ pub enum ConvertMessageError {
UnsupportedVersion,
InvalidDestination,
InvalidToken,
/// The fee asset is not supported for conversion.
Copy link
Member

Choose a reason for hiding this comment

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

No idea where this is used. However, this changes the encoding. Are you aware of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bkchr it is not used anywhere but you make a good point. I don't think removing this adds any benefit really. I could do:

pub enum ConvertMessageError {
    /// The message version is not supported for conversion.
    UnsupportedVersion = 0,
    InvalidDestination = 1,
    InvalidToken = 2,
    /// Keeping the same index as before.
    CannotReanchor = 4,
}

It's ugly, I'd rather just leave it in. @alistair-singh @yrong @vgeddes I'm going to close this PR unless you have reservations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants