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

refactor(audit): shorten CLI's function bodies #215

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

Conversation

pbeza
Copy link
Contributor

@pbeza pbeza commented Feb 14, 2025

This commit addresses finding B1 from the Trail of Bits audit titled 'Functions with too many lines.'

This commit addresses finding B1 from the Trail of Bits audit titled
'Functions with too many lines.'
node/src/cli.rs Outdated Show resolved Hide resolved
node/src/cli.rs Show resolved Hide resolved
node/src/cli.rs Outdated Show resolved Hide resolved
@pbeza pbeza force-pushed the pab/fix/b1-audit-refactor branch from 8c7e4dc to 0142629 Compare February 17, 2025 11:59
@pbeza pbeza force-pushed the pab/fix/b1-audit-refactor branch from 0142629 to 446cef7 Compare February 17, 2025 12:05
@pbeza pbeza requested a review from robin-near February 17, 2025 12:20
@pbeza pbeza force-pushed the pab/fix/b1-audit-refactor branch from 4000691 to 62b6651 Compare February 18, 2025 13:01
@pbeza
Copy link
Contributor Author

pbeza commented Feb 18, 2025

@robin-near, could you review it again, please? BTW, CI checks seem to be flaky – I had to manually re-run the tests to get them to pass.

mpc_client: MpcClient,
auto_abort: tracking::AutoAbortTask<T>,
) -> anyhow::Result<(), JoinError> {
let _auto_abort_metrics = auto_abort;
Copy link
Contributor

Choose a reason for hiding this comment

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

wait I'm not sure why _auto_abort_metrics needs to be in here - we can just have the run() function keep that as a local variable. Don't need to move it in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it here:

pub async fn run(
self,
channel_receiver: mpsc::Receiver<NetworkTaskChannel>,
mut sign_request_receiver: tokio::sync::OwnedMutexGuard<
mpsc::UnboundedReceiver<ChainSignatureRequest>,
>,
chain_txn_sender: mpsc::Sender<ChainSendTransactionRequest>,
) -> anyhow::Result<()> {
let client = self.client.clone();
let metrics_emitter = tracking::spawn("periodically emits metrics", async move {
loop {
client.emit_metrics();
tokio::time::sleep(std::time::Duration::from_secs(5)).await;
}
});

Lemme know if this is what you meant.

node/src/mpc_client.rs Outdated Show resolved Hide resolved
@pbeza pbeza force-pushed the pab/fix/b1-audit-refactor branch from 9a912ee to 972150d Compare February 19, 2025 12:28
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.

2 participants