Skip to content

indexer: identify DEPLOY state changes #252

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

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

Conversation

marcelosalloum
Copy link
Collaborator

What

Identify DEPLOY state changes.

Why

This was still missing. in our state changes mapping

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 22, 2025 00:46
@marcelosalloum marcelosalloum self-assigned this Jul 22, 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 adds support for identifying and tracking DEPLOY state changes in the indexer. The implementation includes adding a DeployerAccountID field to track who deployed contracts, creating a new ContractDeployProcessor to handle contract deployment events, and integrating it into the main indexer pipeline.

Key Changes:

  • Added DeployerAccountID field to the StateChange struct and database schema
  • Created new ContractDeployProcessor to identify contract deployments from Soroban operations
  • Integrated the contract deploy processor into the main indexer workflow

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/indexer/types/types.go Added DeployerAccountID field to StateChange struct
internal/indexer/processors/state_change_builder.go Added WithDeployer method to builder pattern
internal/indexer/processors/contract_deploy.go New processor for handling contract deployment state changes
internal/indexer/processors/contract_deploy_test.go Comprehensive tests for the contract deploy processor
internal/indexer/processors/contracts_test_utils.go Extracted common test utilities for Soroban operations
internal/indexer/processors/contract_operations_test.go Refactored to use extracted test utilities
internal/indexer/indexer.go Integrated contract deploy processor into main workflow
internal/db/migrations/2025-06-10.4-create_indexer_table_state_changes.sql Added deployer_account_id column to database
internal/data/statechanges.go Updated batch insert to handle deployer account ID

@marcelosalloum marcelosalloum marked this pull request as ready for review July 22, 2025 00:48
Copy link
Contributor

@aditya1702 aditya1702 left a comment

Choose a reason for hiding this comment

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

LGTM. It would be nice to have @JakeUrban review the contract parsing once just to make sure it looks correct.

@@ -88,6 +91,13 @@ func (i *Indexer) ProcessTransaction(ctx context.Context, transaction ingest.Led
return fmt.Errorf("processing effects state changes: %w", err)
}
i.Buffer.PushStateChanges(effectsStateChanges)

// 2.2. Index contract deploy state changes
contractDeployStateChanges, err = i.contractDeployProcessor.ProcessOperation(ctx, opParticipants.OpWrapper)
Copy link
Contributor

Choose a reason for hiding this comment

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

Small design idea: since we are using the same interface here, we can instead have a list with these two processors and use a for loop to push the state changes. So something like this:

operationProcessors []OperationProcessorInterface

for _, processor: range b.operationProcessors:
     stateChanges, err := processor.ProcesssOperation...
     ....
     i.Buffer.PushStateChanges(stateChanges)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree with that change but I think it's outside the scope of this PR. It should be done in a dedicated PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I opened a PR here, but in reality this won't scale for more custom contract processors.

This discussion points to the necessity of calling RPC to assert contract interfaces and likely adding a special cache for contracts. This cache can expand a lot and we'll probably need some space-efficient filter, like BloomFilter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm leaving that PR in draft. Feel free to either close it or build on top of it.

Base automatically changed from polishes to main July 22, 2025 21:28
@marcelosalloum marcelosalloum requested a review from JakeUrban July 22, 2025 22:40
opID := op.ID()
builder := NewStateChangeBuilder(op.Transaction.Ledger.LedgerSequence(), op.LedgerClosed.Unix(), op.Transaction.Hash.HexString()).
WithOperationID(opID).
WithCategory(types.StateChangeCategoryContract).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if "contract" is the best category we can use. We're only indexing contract deployments for contracts that are registered as accounts, right? Meaning, the flow would be:

  • wallet generates the contract ID using the wasm/network/salt
  • wallet registers the contract ID with the wallet backend
  • wallet deploys the contract

This to me seems like the contract version of a CreateAccount operation. Which, speaking of which, I don't think we have a state change for an account being created, right? Should we rename this so that creation events for both stellar accounts and contracts result in the same state change? Of course, stellar accounts being created will also result in a credit XLM state change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2025-07-23 at 5 07 35 PM

(G) Account creation is generating these state changes, but for contracts there's no concept of DEBIT/CREDIT in play, it's just fees and deployer so I think the states being changed are different even though they can both be interpreted as account creation.

I'm still inclined to keep some differentiation in G-account and C-account creation because the nature of the accounts are different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, well even if we keep a “Contract”/“Deploy” classification/reason for contract account creation, we don’t have a state change for stellar account creation. Meaning, a client querying for state changes wouldn’t be able to tell when the account was created. They’d have to inspect the operation or transaction. If we have a state change representing account creation for contracts, I think we should have one for classic accounts. Wdyt?

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.

3 participants