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

rustfmt: Run on offers #3577

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Jan 30, 2025

We run the formatter on the offers sub-directory.

@dunxen
Copy link
Contributor

dunxen commented Feb 6, 2025

After the conflicts are resolved here, I don't have a strong opinion on how it's formatted things, so LGTM.

@tnull tnull force-pushed the 2025-01-rustfmt-offers branch from 16a7ba1 to ce2a568 Compare February 6, 2025 15:28
@tnull
Copy link
Contributor Author

tnull commented Feb 6, 2025

After the conflicts are resolved here, I don't have a strong opinion on how it's formatted things, so LGTM.

Resolved conflicts.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

There's about two pending PRs touching this module that should land today or early next week, then lets land this.

/// [`Bolt12Invoice`] for the request if they could be extracted from the metadata.
///
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
pub fn verify_using_metadata<#[cfg(not(c_bindings))] T: secp256k1::Signing>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one might almost be worth rustfmt::skip'ing. Inline cfgs are confusing to read.

() => {
"02080000010000020003"
};
}
macro_rules! tlv3 { () => { "03310266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c0351800000000000000010000000000000002" } }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why didn't this get updated?

"010203e8"
};
}
macro_rules! tlv2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we go ahead and convert these to constants?

macro_rules! tlv3 { () => { "03310266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c0351800000000000000010000000000000002" } }
assert_eq!(
super::root_hash(TlvStream::new(&<Vec<u8>>::from_hex(tlv1!()).unwrap())),
sha256::Hash::from_slice(&<Vec<u8>>::from_hex("b013756c8fee86503a0b4abdab4cddeb1af5d344ca6fc2fa8b6c08938caa6f93").unwrap()).unwrap(),
sha256::Hash::from_slice(
&<Vec<u8>>::from_hex(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you break these up? The Hash/key(from_hex) unwrap unwrap patterns are really good candidates to clean up in a number of places in this file.

@TheBlueMatt
Copy link
Collaborator

Needs rebase, not sure if there's any remaining PRs touching this code? I don't think so.

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

Successfully merging this pull request may close these issues.

3 participants