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(cat-gateway): Fix native asset indexing to be more flexible #1150

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

apskhem
Copy link
Collaborator

@apskhem apskhem commented Nov 6, 2024

Description

  • Renamed the schema.
  • Renamed the response object.
  • Renamed the database mapping object.
  • Fixed asset map to handle list of txos.

Related Issue(s)

Closes #1121

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@apskhem apskhem added the bug Something isn't working label Nov 6, 2024
@apskhem apskhem self-assigned this Nov 6, 2024
@apskhem apskhem marked this pull request as draft November 6, 2024 13:41
@apskhem apskhem added the review me PR is ready for review label Nov 6, 2024
@apskhem apskhem marked this pull request as ready for review November 6, 2024 13:51
Copy link
Contributor

github-actions bot commented Nov 6, 2024

Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

Copy link
Contributor

github-actions bot commented Nov 6, 2024

Test Report | ${\color{lightgreen}Pass: 320/320}$ | ${\color{red}Fail: 0/320}$ |

Copy link
Contributor

github-actions bot commented Nov 6, 2024

Test Report | ${\color{lightgreen}Pass: 320/320}$ | ${\color{red}Fail: 0/320}$ |

@stevenj stevenj marked this pull request as draft November 7, 2024 09:30
@stevenj stevenj added this to the M4: Voting & Delegation milestone Nov 7, 2024
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

See comments, these need to be corrected and then I will conduct another review.

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Test Report | ${\color{lightgreen}Pass: 291/291}$ | ${\color{red}Fail: 0/291}$ |

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Test Report | ${\color{lightgreen}Pass: 320/320}$ | ${\color{red}Fail: 0/320}$ |

@apskhem apskhem requested a review from stevenj November 7, 2024 15:39
@apskhem apskhem marked this pull request as ready for review November 8, 2024 13:43
@apskhem apskhem marked this pull request as draft November 11, 2024 12:33
@apskhem apskhem marked this pull request as ready for review November 11, 2024 12:53
Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 320/320}$ | ${\color{red}Fail: 0/320}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 320/320}$ | ${\color{red}Fail: 0/320}$ |

Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

From the original issue it is also telling to the change the asset value type to i128, which I dont see in this PR.
Is it really needed, or its something we dont want to do ?

amount: num_bigint::BigInt::ZERO,
});
entry.amount += row.value;
.or_insert_with(Vec::new);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite get it this change.
Could you elaborate what we are doing here actually ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working review me PR is ready for review
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

[Bug]: Native Asset indexing and endpoint response has issues that need correcting.
4 participants