Skip to content

{indexer, services.rpc}: optimize the usage of TransactionOperationWrapper and fix RPC client error handling #248

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

Merged
merged 3 commits into from
Jul 22, 2025

Conversation

marcelosalloum
Copy link
Collaborator

@marcelosalloum marcelosalloum commented Jul 17, 2025

What

  • Use the TransactionOperationWrapper object to prevent rebuilding it multiple times. Now it's only built once per operation instead of once per operation and once per state change.
  • Use pointers for TransactionOperationWrapper, so we prevent copying it all the time and decrease the memory allocation, which is relevant for high throughput
  • Fix RPC client so it can properly parse the response body when the response status code is not 200.

Why

Bug fixes and Performance improvements.

Checklist

PR Structure

  • It is not possible to break this PR down into smaller PRs.
  • This PR does not mix refactoring changes with feature changes.
  • This PR's title starts with name of package that is most changed in the PR, or all if the changes are broad or impact many packages.

Thoroughness

  • This PR adds tests for the new functionality or fixes.
  • All updated queries have been tested (refer to this check if the data set returned by the updated query is expected to be same as the original one).

Release

  • This is not a breaking change.
  • This is ready to be tested in development.
  • The new functionality is gated with a feature flag if this is not ready for production.

@marcelosalloum marcelosalloum requested a review from Copilot July 17, 2025 22:06
@marcelosalloum marcelosalloum self-assigned this Jul 17, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes the usage of TransactionOperationWrapper objects and fixes RPC client error handling. The changes focus on performance improvements through memory allocation optimization and proper error handling for non-200 HTTP responses.

  • Convert TransactionOperationWrapper usage from value types to pointer types to avoid unnecessary copying
  • Fix RPC client to properly handle and parse response bodies when status codes are not 200
  • Refactor operation processing interfaces to accept TransactionOperationWrapper pointers

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/services/rpc_service.go Adds HTTP status code validation and error handling for non-200 responses
internal/services/rpc_service_test.go Updates test mocks to include StatusCode field for proper testing
internal/indexer/processors/participants.go Changes TransactionOperationWrapper from value to pointer type
internal/indexer/processors/effects.go Updates ProcessOperation to accept pointer and removes redundant wrapper creation
internal/indexer/processors/contract_operations.go Updates function signatures to use pointer types
internal/indexer/indexer.go Updates calls to use new pointer-based interface
internal/indexer/mocks.go Updates mock interface to match new pointer-based signature
Multiple test files Updates test code to create and use TransactionOperationWrapper pointers

@marcelosalloum marcelosalloum marked this pull request as ready for review July 17, 2025 22:08
@marcelosalloum marcelosalloum merged commit 206fd7c into main Jul 22, 2025
8 checks passed
@marcelosalloum marcelosalloum deleted the polishes branch July 22, 2025 21:28
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