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

chore(erc20): add erc20 extension approval tests #2005

Merged
merged 25 commits into from Nov 7, 2023

Conversation

MalteHerrmann
Copy link
Contributor

@MalteHerrmann MalteHerrmann commented Nov 6, 2023

Description

This PR adds unit tests for the ERC20 extension approvals.

Additionally, some authorization related methods were added to the integration test utils.


Closes ENG-2285

Copy link

linear bot commented Nov 6, 2023

precompiles/erc20/approve.go Fixed Show resolved Hide resolved
Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #2005 (508fc5c) into main (4c251e3) will increase coverage by 0.56%.
Report is 2 commits behind head on main.
The diff coverage is 80.00%.

❗ Current head 508fc5c differs from pull request most recent head 6b6253a. Consider uploading reports for the commit 6b6253a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2005      +/-   ##
==========================================
+ Coverage   69.14%   69.71%   +0.56%     
==========================================
  Files         334      334              
  Lines       24837    24830       -7     
==========================================
+ Hits        17173    17309     +136     
+ Misses       6780     6627     -153     
- Partials      884      894      +10     
Files Coverage Δ
precompiles/erc20/approve.go 82.46% <100.00%> (+82.46%) ⬆️
precompiles/erc20/query.go 8.49% <60.00%> (+8.49%) ⬆️

precompiles/erc20/approve.go Outdated Show resolved Hide resolved
precompiles/erc20/approve.go Show resolved Hide resolved
@MalteHerrmann MalteHerrmann marked this pull request as ready for review November 6, 2023 21:59
@MalteHerrmann MalteHerrmann requested a review from a team as a code owner November 6, 2023 21:59
@MalteHerrmann MalteHerrmann requested review from Vvaradinov and removed request for a team November 6, 2023 21:59
Copy link
Contributor

@Vvaradinov Vvaradinov left a comment

Choose a reason for hiding this comment

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

LGTM. Pending the open questions regarding authorization handling of denoms and deleting on 0 limit left.

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Good findings @MalteHerrmann! left some comments

precompiles/erc20/approve.go Fixed Show resolved Hide resolved
precompiles/erc20/approve_test.go Show resolved Hide resolved
precompiles/erc20/approve_test.go Show resolved Hide resolved
precompiles/erc20/approve_test.go Show resolved Hide resolved
precompiles/erc20/approve_test.go Show resolved Hide resolved
@fedekunze fedekunze marked this pull request as draft November 7, 2023 11:06
@MalteHerrmann MalteHerrmann marked this pull request as ready for review November 7, 2023 12:14
@MalteHerrmann
Copy link
Contributor Author

After discussion the open points and adjustments will be handled on a follow-up PR to make reviews cleaner.

@MalteHerrmann MalteHerrmann enabled auto-merge (squash) November 7, 2023 12:16
@MalteHerrmann MalteHerrmann merged commit ae04f14 into main Nov 7, 2023
27 of 28 checks passed
@MalteHerrmann MalteHerrmann deleted the malte/erc20-extension-approve-tests branch November 7, 2023 12:47
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.

None yet

3 participants