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

[rust] handle burn properly for partial burns #327

Merged
merged 4 commits into from
Mar 21, 2024
Merged

[rust] handle burn properly for partial burns #327

merged 4 commits into from
Mar 21, 2024

Conversation

bowenyang007
Copy link
Collaborator

Summary

This closes the loop on the messy logic that is burning NFTs:
There could be a complete burn or a partial burn.

  • In the complete burn case, we don’t have anything so we either have the previous_owner field from the new burn event, or have to do a db lookup
  • In the partial burn case, we assumed (wrongly) that ObjectCore will be there.

This PR fixes the partial burn case.

Backfill

First burn event
Testnet: 835574507
Mainnet: 151521169

Testing

NFT burnt!
image

@bowenyang007 bowenyang007 force-pushed the fix_burn branch 2 times, most recently from 2c5b71a to 9065ddc Compare March 21, 2024 02:55
@bowenyang007 bowenyang007 changed the base branch from main to copy_v1.10 March 21, 2024 02:56
Base automatically changed from copy_v1.10 to main March 21, 2024 03:12
@@ -296,20 +339,19 @@ impl TokenOwnershipV2 {
query_retries: u32,
query_retry_delay_ms: u64,
) -> anyhow::Result<Option<(Self, CurrentTokenOwnershipV2)>> {
let token_address = standardize_address(&write_resource.address.to_string());
if let Some(burn_event) = tokens_burned.get(&token_address) {
if let Some(burn_event) = tokens_burned.get(token_address) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe call standardize_address() on tokens_burned here too in case we use the helper fn anywhere else in the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok yea good poitn.

@bowenyang007 bowenyang007 merged commit f536b28 into main Mar 21, 2024
7 checks passed
@bowenyang007 bowenyang007 deleted the fix_burn branch March 21, 2024 21:20
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