Skip to content

feat(cast): Include recover_authority when logging SignedAuthorization #10349

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
Merged
merged 5 commits into from
Apr 23, 2025
Merged

feat(cast): Include recover_authority when logging SignedAuthorization #10349

merged 5 commits into from
Apr 23, 2025

Conversation

naijauser
Copy link
Contributor

Closes 10100

Hello @mattsse @yash-atreya,

Please review, thanks!

zerosnacks
zerosnacks reviewed Apr 22, 2025
View reviewed changes
mattsse
mattsse previously requested changes Apr 22, 2025
View reviewed changes
@grandizzy grandizzy added T-feature Type: feature C-cast Command: cast labels Apr 23, 2025
@grandizzy grandizzy requested review from zerosnacks and mattsse April 23, 2025 05:03
yash-atreya
yash-atreya previously approved these changes Apr 23, 2025
View reviewed changes
Copy link
Member

@yash-atreya yash-atreya left a comment

Choose a reason for hiding this comment

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

lgtm!

mattsse
mattsse requested changes Apr 23, 2025
View reviewed changes
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

smol naming nit,

we could also name this just authority, this being an address would be enough context imo

wdyt @grandizzy

@Yasersarhadikarbasak
Copy link

عالی ممنونم

@Yasersarhadikarbasak
Copy link

عالی 👍🏻

@grandizzy
Copy link
Collaborator

smol naming nit,

we could also name this just authority, this being an address would be enough context imo

wdyt @grandizzy

yep, agree, @naijauser let's rename to authority and apply recoverAuthority -> recoveredAuthority suggestions, then good to go. thank you!

@naijauser
Copy link
Contributor Author

smol naming nit,
we could also name this just authority, this being an address would be enough context imo
wdyt @grandizzy

yep, agree, @naijauser let's rename to authority and apply recoverAuthority -> recoveredAuthority suggestions, then good to go. thank you!

Updated! @mattsse @grandizzy

mattsse
mattsse approved these changes Apr 23, 2025
View reviewed changes
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

@grandizzy grandizzy enabled auto-merge (squash) April 23, 2025 13:22
@grandizzy grandizzy merged commit 77aec59 into foundry-rs:master Apr 23, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry Apr 23, 2025
@naijauser naijauser deleted the include-recover-authority branch April 23, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@DaniPopes DaniPopes Awaiting requested review from DaniPopes DaniPopes is a code owner

@klkvr klkvr Awaiting requested review from klkvr klkvr is a code owner

@grandizzy grandizzy Awaiting requested review from grandizzy grandizzy is a code owner

@zerosnacks zerosnacks Awaiting requested review from zerosnacks zerosnacks is a code owner

@yash-atreya yash-atreya Awaiting requested review from yash-atreya yash-atreya is a code owner

Assignees
No one assigned
Labels
C-cast Command: cast T-feature Type: feature
Projects
Foundry
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(cast): include recover_authority when logging SignedAuthorization
6 participants
@naijauser @Yasersarhadikarbasak @grandizzy @mattsse @yash-atreya @zerosnacks