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

Remediate Trevor's comments #2835

Merged
merged 6 commits into from
Oct 24, 2023
Merged

Remediate Trevor's comments #2835

merged 6 commits into from
Oct 24, 2023

Conversation

aroralanuk
Copy link
Contributor

@aroralanuk aroralanuk commented Oct 23, 2023

Description

Addressing @tkporter 's comments from here: #2733

  • Fix msg.value exploit on AbstractMessageIdAuthorizedIsm
  • Switched authorizedHook and ism from address to bytes32 for multi VM extensibility
  • Added enum to hooks interface for use by clients

Drive-by changes

none

Related issues

none

Backward compatibility

No

Testing

Unit

@aroralanuk aroralanuk changed the title Remediate trevor comments Remediate Trevor's comments Oct 23, 2023
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #2835 (e1b3b68) into v3 (49fc06e) will increase coverage by 0.46%.
The diff coverage is 88.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v3    #2835      +/-   ##
==========================================
+ Coverage   62.07%   62.53%   +0.46%     
==========================================
  Files          99       99              
  Lines        1002     1017      +15     
  Branches      104      106       +2     
==========================================
+ Hits          622      636      +14     
- Misses        343      344       +1     
  Partials       37       37              
Components Coverage Δ
core 50.00% <ø> (ø)
hooks 69.28% <86.66%> (+1.34%) ⬆️
isms 69.78% <100.00%> (+1.12%) ⬆️
token 55.44% <ø> (ø)
middlewares 73.17% <ø> (ø)

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

please populate the description with more specific explanations

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

please populate the description and title with more specific explanations of changes
feel free to copy in @tkporter's comments

@yorhodes yorhodes self-requested a review October 24, 2023 00:59
@yorhodes yorhodes merged commit e90ae5a into v3 Oct 24, 2023
@yorhodes yorhodes deleted the remediate-trevor-comments branch October 24, 2023 01:00
@yorhodes yorhodes mentioned this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants