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.

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

Add E2E test support for ERC20/ERC721 #611

Merged
merged 5 commits into from Mar 18, 2022
Merged

Add E2E test support for ERC20/ERC721 #611

merged 5 commits into from Mar 18, 2022

Conversation

awrichar
Copy link
Contributor

@awrichar
Store TXType in token events
deb6cef 
Because some token transactions may generate events of multiple types
(such as a transfer triggering approval changes as a side effect), the
transaction type must be stored instead of being inferred from the event.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar
Add E2E test support for ERC20/ERC721
f94f5dc 
Resolves hyperledger#564

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar mentioned this pull request Mar 18, 2022
Closed
@awrichar awrichar changed the base branch from erc20 to main March 18, 2022 03:38
@awrichar
Copy link
Contributor Author

I attempted to add ERC20/ERC721 tokens to the E2E matrix for GitHub PRs - will see if it works and runs here. Open to whether this is needed or not (obviously we only want to run a subset of all combinations on every PR).

@nguyer nguyer closed this Mar 18, 2022
@nguyer nguyer reopened this Mar 18, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #611 (6dc62e6) into main (2e464e7) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #611   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          305       305           
  Lines        18196     18266   +70     
=========================================
+ Hits         18196     18266   +70
Impacted Files Coverage Δ
internal/database/sqlcommon/tokenpool_sql.go 100.00% <100.00%> (ø)
internal/events/token_pool_created.go 100.00% <100.00%> (ø)
internal/tokens/fftokens/fftokens.go 100.00% <100.00%> (ø)
internal/database/sqlcommon/chart_sql.go 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e464e7...6dc62e6. Read the comment docs.

nguyer
nguyer suggested changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@nguyer nguyer left a comment

Choose a reason for hiding this comment

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

One minor thing on the GitHub action, but other than that, looks good

@nguyer
Copy link
Contributor

nguyer commented Mar 18, 2022

And there's a failing E2E test 😕

@awrichar
Copy link
Contributor Author

It's weird that Fabric + ERC1155 even executes (and passes?)... does it just ignore the token provider? I know it's definitely not actually using it.

And yea the failures in ERC20 are expected until the dependent PR noted is merged and tagged in the manifest.

@awrichar
Copy link
Contributor Author

Opened hyperledger/firefly-cli#162 to track the fact that -t is silently ignored by Fabric stacks.

@awrichar
Store token pool info
509b733 
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar
Update manifest for firefly-tokens-erc20-erc721
c632456 
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar
Remove fabric+erc1155 from E2E matrix
f90e90a 
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar merged commit 7f196be into hyperledger:main Mar 18, 2022
@awrichar awrichar deleted the erc20 branch March 18, 2022 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nguyer nguyer nguyer approved these changes

@peterbroadhurst peterbroadhurst Awaiting requested review from peterbroadhurst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@awrichar @codecov-commenter @nguyer