Conversation
Summary of ChangesHello @S4mmyb, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new indexing capability to track ecocredit transfers within the system. By capturing and storing Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5ed2f0a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request introduces indexing for ecocredit credit transfers, a valuable addition for tracking credit movements. However, the implementation contains two high-severity data integrity issues: a logic error that causes data corruption when a single message emits multiple transfer events, and precision loss due to the use of floating-point arithmetic for high-precision credit amounts, making the indexed transfer data unreliable for financial auditing or ownership tracking. Additionally, there are opportunities for performance and robustness improvements in index_transfers.py.
blushi
left a comment
There was a problem hiding this comment.
Maybe we could use a view backed by indexes instead of an additional table to save on db storage costs without too much impact on performance?
- Replace table-based indexing with view that extracts from msg.data - Solves multi-batch correlation issue by using source JSON structure - Aggregates duplicate batch_denoms within same MsgSend - Removes index_transfers.py polling process (no longer needed) - Handles empty string amounts properly with CASE statements - Adds indexes on msg table for view query performance This approach is cleaner and more maintainable than event-based polling.
Good idea - honestly ended up being a lot simpler too. Lmk what you think now. I tested a few transactions, but this was the one for the edge case: |
|
hey @blushi , do you think I could get a review on this? |
|
@S4mmyb You'll have to submit a PR from dev to main for the changes to reflect on prod. |
Summary
Adds indexing support for ecocredit credit transfers. The indexer now tracks
EventTransferevents fromMsgSendtransactions, storing transfer details in a newtransferstable. This enables tracking all credit movements between accounts, which is essential for:What changed
transferstable: Stores individual credit transfer records with:index_transfers.py: ParsesEventTransferevents and indexes themMsgSendonly (EventTransfer can emit from other message types)utils.py: AddedtransferstoTABLE_EVENT_NAMES_MAPwith both v1 event types; v1alpha was skipped because mainnet has no MsgSend during v1alphamain.py: Addedindex_transfers()call to start the transfers indexer process000008.sql: Creates the transfers table with proper indexes and foreign key constraintstest_index_transfers.shfor basic smoke testingWhat was tested
regen-upgradetestnetMsgSendcontaining credit transfers individually, and bundledEventTransferevents were correctly parsed and inserted into transfers table