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

fix token claims #627

Merged
merged 2 commits into from
Dec 6, 2024
Merged

fix token claims #627

merged 2 commits into from
Dec 6, 2024

Conversation

rtso
Copy link
Collaborator

@rtso rtso commented Dec 6, 2024

Description

When a token gets claimed, processor needs to know the previous owner for token_activities_v2. Currently the processor gets it from PendingClaimsResource but in this case the resource wasn't found in the transaction and breaks the processor. This PR tries to get it previous owner from the TokenClaim event instead of the table metadata.

Testing

Screenshot 2024-12-06 at 12 44 57 PM

Copy link
Collaborator Author

rtso commented Dec 6, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

@rtso rtso force-pushed the 11-13-fix_claims branch 4 times, most recently from 0da8dc2 to f9cba7c Compare December 6, 2024 20:45
@rtso rtso changed the title fix claims fix token claims Dec 6, 2024
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.

Project coverage is 49.3%. Comparing base (b823242) to head (323b0b1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...rc/db/postgres/models/token_models/token_claims.rs 80.0% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #627     +/-   ##
=======================================
+ Coverage   49.2%   49.3%   +0.1%     
=======================================
  Files        195     195             
  Lines      25665   25701     +36     
=======================================
+ Hits       12629   12678     +49     
+ Misses     13036   13023     -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rtso rtso marked this pull request as ready for review December 6, 2024 20:57
@rtso rtso force-pushed the 11-13-fix_claims branch from f9cba7c to 8b2f096 Compare December 6, 2024 21:05
@rtso rtso requested review from lightmark and a team December 6, 2024 21:23
@rtso rtso force-pushed the 11-13-fix_claims branch from 1921fd3 to 323b0b1 Compare December 6, 2024 21:23
Copy link
Contributor

@dermanyang dermanyang left a comment

Choose a reason for hiding this comment

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

code lgtm. can you explain the testing screenshot please? i don't understand how it tests the new logic

Copy link
Collaborator Author

rtso commented Dec 6, 2024

@dermanyang token v2 processor is failing in devnet with version 19922017 so to test the fix, I ran it with the fix locally and checked that 1) the processor didn't panic and 2) the data is correct. also added an integration test for this case.

@rtso rtso merged commit c442b42 into main Dec 6, 2024
11 checks passed
@rtso rtso deleted the 11-13-fix_claims branch December 6, 2024 21:41
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