chore: decouple Node releases from SQS #8886
Conversation
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
|
Note: I am aware of failing |
WalkthroughThis pull request introduces a comprehensive refactoring of the Osmosis ingest package, primarily focusing on migrating type definitions and import paths from the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (16)
ingest/types/cosmwasmpool/alloy_transmuter.go (1)
20-20: Fix typo in comment: 'Tranmuter' should be 'Transmuter'There's a spelling error in the comment on line 20. Correcting this improves code readability and aligns with standard terminology.
Apply this diff to fix the typo:
-// Tranmuter Alloyed Data, since v3.0.0 +// Transmuter Alloyed Data, since v3.0.0ingest/types/cosmwasmpool/orderbook.go (3)
29-38: Consider using an explicit enum forOrderbookDirectionandRoundingDirectionCurrently,
OrderbookDirectionandRoundingDirectionare defined asbool, which can be less intuitive and may lead to confusion. Using an explicitiota-based enum improves code clarity and type safety.Apply this diff to define explicit enums:
type OrderbookDirection int const ( - BID OrderbookDirection = true - ASK OrderbookDirection = false + BID OrderbookDirection = iota + ASK ) type RoundingDirection int const ( - ROUND_UP RoundingDirection = true - ROUND_DOWN RoundingDirection = false + ROUND_UP RoundingDirection = iota + ROUND_DOWN )
60-66: SimplifyIterationStepmethod by removing redundant error returnThe
IterationStepmethod always returns a valid integer and never actually returns an error. Simplifying the method signature enhances readability.Apply this diff to simplify the method:
-func (d *OrderbookDirection) IterationStep() (int, error) { +func (d *OrderbookDirection) IterationStep() int { if *d { // BID - return -1, nil + return -1 } else { // ASK - return 1, nil + return 1 } }Update the calling function accordingly:
func CalcAmountInToExhaustOrderbookLiquidity(directionIn OrderbookDirection, startingIndex int, ticks []OrderbookTick) (osmomath.BigDec, error) { directionOut := directionIn.Opposite() - directionOutIterationStep, err := directionOut.IterationStep() - if err != nil { - return osmomath.ZeroBigDec(), err - } + directionOutIterationStep := directionOut.IterationStep()
123-130: SimplifyGetStartTickIndexmethod by removing redundant error returnThe
GetStartTickIndexmethod does not return an error in any case. Removing the error return simplifies the code.Apply this diff to update the method:
-func (d *OrderbookData) GetStartTickIndex(direction OrderbookDirection) (int, error) { +func (d *OrderbookData) GetStartTickIndex(direction OrderbookDirection) int { if direction == ASK { - return d.NextAskTickIndex, nil + return d.NextAskTickIndex } else { // BID - return d.NextBidTickIndex, nil + return d.NextBidTickIndex } }Ensure any calls to this method are updated appropriately.
ingest/sqs/domain/mocks/grpc_client_mock.go (1)
5-5: Remove unused import ofsqsdomainpackageThe
sqsdomainpackage is imported but no longer used after the changes. Removing unused imports cleans up the code.Apply this diff to remove the unused import:
import ( "context" - "github.com/osmosis-labs/osmosis/v28/ingest/sqs/domain" "github.com/osmosis-labs/osmosis/v28/ingest/types" )ingest/types/passthroughdomain/passthrough_pools_data.go (1)
4-8: LGTM! Consider adding an interface.The wrapper types are well-structured with proper JSON tags and clear documentation. The use of embedded structs and
omitemptyfor optional fields is appropriate.Consider defining an interface that both wrappers implement to enable generic handling of status-wrapped pool data:
type PoolDataStatusWrapper interface { IsStaleStatus() bool IsErrorStatus() bool }Also applies to: 11-15
ingest/types/errors.go (1)
13-19: Enhance error message with valid values.The error message for invalid direction could be more helpful by explicitly stating the valid values.
Consider updating the error message to be more specific:
- return fmt.Sprintf("orderbook pool direction (%d) is invalid; must be either -1 or 1", e.Direction) + return fmt.Sprintf("orderbook pool direction (%d) is invalid; valid values are: -1 (sell), 1 (buy)", e.Direction)ingest/types/cosmwasmpool/errors.go (1)
5-13: Add validation for empty denom values.Consider adding validation in the constructor or Error() method to handle empty denom values gracefully.
Example implementation:
func NewOrderbookUnsupportedDenomError(denom, quoteDenom, baseDenom string) OrderbookUnsupportedDenomError { if denom == "" { denom = "<empty>" } if quoteDenom == "" { quoteDenom = "<empty>" } if baseDenom == "" { baseDenom = "<empty>" } return OrderbookUnsupportedDenomError{ Denom: denom, QuoteDenom: quoteDenom, BaseDenom: baseDenom, } }ingest/types/json/sqs_json.go (2)
4-4: Remove TODO commentThe TODO comment in the import statement should be removed or replaced with actual documentation.
28-34: Consider wrapping errors for better contextThe implementation looks good, but consider wrapping errors with additional context to help with debugging. This is especially important since this is a core JSON package that will be used throughout the application.
func Marshal(v interface{}) ([]byte, error) { - return jsonLib.Marshal(v) + b, err := jsonLib.Marshal(v) + if err != nil { + return nil, fmt.Errorf("json.Marshal: %w", err) + } + return b, nil } func Unmarshal(data []byte, v interface{}) error { - return jsonLib.Unmarshal(data, v) + if err := jsonLib.Unmarshal(data, v); err != nil { + return fmt.Errorf("json.Unmarshal: %w", err) + } + return nil }ingest/types/passthroughdomain/pool_fees.go (1)
18-36: Add validation in UnmarshalJSONConsider adding validation for required fields and numerical values in the UnmarshalJSON implementation.
func (p *PoolFee) UnmarshalJSON(data []byte) error { type Alias PoolFee temp := &struct { PoolID string `json:"pool_id"` *Alias }{ Alias: (*Alias)(p), } if err := json.Unmarshal(data, temp); err != nil { return err } + // Validate required fields + if temp.PoolID == "" { + return fmt.Errorf("pool_id is required") + } + + // Validate numerical values + if temp.Volume24h < 0 || temp.Volume7d < 0 { + return fmt.Errorf("volume cannot be negative") + } + if temp.FeesSpent24h < 0 || temp.FeesSpent7d < 0 { + return fmt.Errorf("fees spent cannot be negative") + } + p.PoolID = temp.PoolID return nil }ingest/types/passthroughdomain/pool_apr.go (1)
32-56: Consider enhancing error handling with more context.While the error handling is good, consider providing more context in the error message:
- return fmt.Errorf("invalid pool_id: %w", err) + return fmt.Errorf("failed to parse pool_id '%s': %w", temp.PoolID, err)ingest/types/taker_fee.go (1)
58-83: Remove unnecessary newlines.There are unnecessary leading newlines in several functions.
-func (tfm TakerFeeMap) Has(denom0, denom1 string) bool { - +func (tfm TakerFeeMap) Has(denom0, denom1 string) bool { _, found := tfm[DenomPair{Denom0: denom0, Denom1: denom1}] return found } -func (tfm TakerFeeMap) GetTakerFee(denom0, denom1 string) osmomath.Dec { - +func (tfm TakerFeeMap) GetTakerFee(denom0, denom1 string) osmomath.Dec { takerFee, found := tfm[DenomPair{Denom0: denom0, Denom1: denom1}] -func (tfm TakerFeeMap) SetTakerFee(denom0, denom1 string, takerFee osmomath.Dec) { - +func (tfm TakerFeeMap) SetTakerFee(denom0, denom1 string, takerFee osmomath.Dec) { tfm[DenomPair{Denom0: denom0, Denom1: denom1}] = takerFee }🧰 Tools
🪛 GitHub Check: Run golangci-lint
[failure] 58-58:
unnecessary leading newline (whitespace)
[failure] 67-67:
unnecessary leading newline (whitespace)
[failure] 80-80:
unnecessary leading newline (whitespace)proto/osmosis/ingest/v1beta1/ingest.proto (3)
7-12: Enhance service documentationWhile the service purpose is clear, consider adding more details to the ProcessBlock RPC documentation:
- Expected behavior on success/failure
- Any rate limiting or size constraints
- Whether it's idempotent
// SQSIngester is a a data ingester from an Osmosis node to // the sidecar query server. service SQSIngester { - // ProcessBlock processes a block from the Osmosis node. + // ProcessBlock processes a block from the Osmosis node. + // It is idempotent and can be safely retried. + // Returns an empty response on success, or a gRPC error on failure. rpc ProcessBlock(ProcessBlockRequest) returns (ProcessBlockReply) {} }
15-26: Consider schema evolution and validation strategyThe use of
bytesfields for all models makes the schema opaque and harder to validate. Consider:
- Using specific message types instead of bytes for better schema evolution
- Adding version fields to help with future migrations
- Adding pool type enum to explicitly check tick_model validity
Also, note that in protobuf, fields are never
nil- they're either present or absent. The comment abouttick_modelbeingnilshould be updated.message PoolData { + // The version of the pool data format. + uint32 version = 1; + + // The type of the pool. + enum PoolType { + POOL_TYPE_UNSPECIFIED = 0; + POOL_TYPE_CONCENTRATED = 1; + POOL_TYPE_STANDARD = 2; + } + PoolType pool_type = 2; + // ChainModel is the chain representation model of the pool. - bytes chain_model = 1; + bytes chain_model = 3; // SqsModel is additional pool data used by the sidecar query server. - bytes sqs_model = 2; + bytes sqs_model = 4; // TickModel is the tick data of a concentrated liquidity pool. - // This field is only valid and set for concentrated pools. It is nil + // This field is only present for concentrated pools and should be ignored // otherwise. - bytes tick_model = 3; + bytes tick_model = 5; }
31-43: Add field constraints and consider structured taker fees
Consider adding field validation:
- Non-zero block height
- Maximum size for taker_fees_map
- Maximum number of pools
Similar to PoolData, consider using a structured message for taker_fees_map instead of bytes.
Consider adding metadata to ProcessBlockReply for monitoring/debugging:
- Number of pools processed
- Processing timestamp
- Status details
message ProcessBlockRequest { // block height is the height of the block being processed. - uint64 block_height = 1; + uint64 block_height = 1 [(validate.rules).uint64.gt = 0]; // taker_fees_map is the map of taker fees for the block. - bytes taker_fees_map = 2; + TakerFeesMap taker_fees_map = 2; // pools in the block. - repeated PoolData pools = 3; + repeated PoolData pools = 3 [(validate.rules).repeated.max_items = 1000]; } +// TakerFeesMap represents the taker fees collected in a block +message TakerFeesMap { + message Fee { + string denom = 1; + string amount = 2 [(validate.rules).string.pattern = "^[0-9]+$"]; + } + map<string, Fee> fees = 1; +} // The response after completing the block processing. -message ProcessBlockReply {} +message ProcessBlockReply { + // Number of pools successfully processed + uint32 processed_pools_count = 1; + // Server-side processing timestamp + google.protobuf.Timestamp processed_at = 2; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumingest/types/proto/types/ingest.pb.gois excluded by!**/*.pb.go,!**/*.pb.go
📒 Files selected for processing (27)
ingest/indexer/service/indexer_streaming_service_test.go(4 hunks)ingest/sqs/domain/ingester.go(3 hunks)ingest/sqs/domain/mocks/grpc_client_mock.go(2 hunks)ingest/sqs/domain/mocks/pool_transformer_mock.go(1 hunks)ingest/sqs/pools/transformer/alloy_transmuter.go(1 hunks)ingest/sqs/pools/transformer/alloy_transmuter_test.go(1 hunks)ingest/sqs/pools/transformer/export_test.go(2 hunks)ingest/sqs/pools/transformer/orderbook.go(1 hunks)ingest/sqs/pools/transformer/orderbook_test.go(1 hunks)ingest/sqs/pools/transformer/pool_transformer.go(5 hunks)ingest/sqs/pools/transformer/pool_transformer_test.go(11 hunks)ingest/sqs/pools/transformer/taker_fee.go(2 hunks)ingest/sqs/pools/transformer/taker_fee_test.go(8 hunks)ingest/sqs/service/grpc_client.go(3 hunks)ingest/sqs/service/sqs_streaming_service_test.go(2 hunks)ingest/types/cosmwasmpool/alloy_transmuter.go(1 hunks)ingest/types/cosmwasmpool/errors.go(1 hunks)ingest/types/cosmwasmpool/model.go(1 hunks)ingest/types/cosmwasmpool/orderbook.go(1 hunks)ingest/types/errors.go(1 hunks)ingest/types/json/sqs_json.go(1 hunks)ingest/types/passthroughdomain/passthrough_pools_data.go(1 hunks)ingest/types/passthroughdomain/pool_apr.go(1 hunks)ingest/types/passthroughdomain/pool_fees.go(1 hunks)ingest/types/pools.go(1 hunks)ingest/types/taker_fee.go(1 hunks)proto/osmosis/ingest/v1beta1/ingest.proto(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- ingest/sqs/pools/transformer/orderbook.go
- ingest/sqs/pools/transformer/orderbook_test.go
- ingest/sqs/pools/transformer/alloy_transmuter_test.go
🧰 Additional context used
🪛 GitHub Check: Run golangci-lint
ingest/types/cosmwasmpool/model.go
[failure] 17-17:
const alloyTransmuterMinVersionStr is unused (unused)
ingest/types/taker_fee.go
[failure] 58-58:
unnecessary leading newline (whitespace)
[failure] 67-67:
unnecessary leading newline (whitespace)
[failure] 80-80:
unnecessary leading newline (whitespace)
🪛 GitHub Actions: Lint
ingest/types/cosmwasmpool/model.go
[error] 17-17: const alloyTransmuterMinVersionStr is unused
ingest/types/pools.go
[warning] 5-5: File is not goimports-ed
🪛 GitHub Actions: Spell Check
ingest/types/cosmwasmpool/alloy_transmuter.go
[error] 39-39: Spelling error: 'limitin' should be 'limiting' or 'limit in'
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (34)
ingest/types/errors.go (1)
5-11: LGTM! Clear and informative error message.The error type and message for concentrated pool tick model is well-structured and informative.
ingest/sqs/domain/mocks/pool_transformer_mock.go (1)
12-13: LGTM! Clean mock implementation.The mock is well-structured with proper interface implementation and type updates.
Also applies to: 20-22
ingest/types/cosmwasmpool/errors.go (2)
15-21: LGTM! Clear error message.The error type and message for duplicated denom is well-structured.
23-30: Verify OrderbookDirection.String() implementation.The error message relies on OrderbookDirection.String() for direction representation.
Let's verify the String() implementation:
✅ Verification successful
OrderbookDirection.String() implementation verified and correct
The String() method properly converts the direction to "BID" or "ASK" strings, which will result in clear and accurate error messages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for OrderbookDirection type and String() method ast-grep --pattern 'type OrderbookDirection $_ { $$$ } func (OrderbookDirection) String() string { $$$ }'Length of output: 112
Script:
#!/bin/bash # Search for OrderbookDirection type definition echo "=== Searching for OrderbookDirection type ===" rg "type\s+OrderbookDirection" -A 5 echo -e "\n=== Searching for OrderbookDirection String method ===" rg "func.*OrderbookDirection.*String\(\)" -A 5 echo -e "\n=== Backup search for any OrderbookDirection references ===" rg "OrderbookDirection" --type goLength of output: 2709
ingest/sqs/pools/transformer/taker_fee.go (1)
17-17: LGTM! Clean migration to new types packageThe function signature has been properly updated to use the new types package while maintaining the same robust functionality.
ingest/sqs/domain/ingester.go (1)
18-18: LGTM! Clean interface updatesThe interface method signatures have been properly updated to use the new types package while maintaining the same contract and behavior.
Also applies to: 30-30
ingest/types/passthroughdomain/pool_apr.go (1)
17-30: Well-structured data model with clear documentation!The struct definition is clean, with proper documentation for each field and appropriate JSON tags.
ingest/types/cosmwasmpool/model.go (1)
40-58: Well-designed model with clear documentation!The
CosmWasmPoolModelandCosmWasmPoolDatastructs are well-designed with clear documentation explaining the tagged union pattern.ingest/types/taker_fee.go (2)
11-19: Well-designed types with clear documentation!The
DenomPairandTakerFeeMaptypes are well-designed with clear documentation about the bi-directional taker fee support.
24-54: Robust JSON marshaling implementation!The JSON marshaling and unmarshaling implementation is robust with proper error handling and clear string representation of denom pairs.
ingest/sqs/pools/transformer/export_test.go (1)
29-33: Clean type reference updates!The changes correctly update the type references from
sqsdomaintotypes, aligning with the PR objective of decoupling from SQS.ingest/types/pools.go (4)
16-36: Well-designed interface with clear method signatures.The
PoolIinterface is well-structured with comprehensive documentation for each method.
40-44: Clean and focused struct definition.The
TickModelstruct effectively captures the required fields with appropriate JSON tags.
46-56: Well-documented struct with clear field purposes.The
SQSPoolstruct includes helpful comments explaining when specific fields are needed.
58-64: Comprehensive wrapper implementation.The
PoolWrapperstruct effectively implements thePoolIinterface with all necessary fields.ingest/sqs/service/grpc_client.go (3)
11-12: Clean import path updates.The import paths have been correctly updated to use the new
typespackage.
43-43: Correctly updated method signature.The
PushDatamethod signature has been properly updated to usetypes.PoolIandtypes.TakerFeeMap.
112-112: Correctly updated method signature.The
marshalPoolsmethod signature has been properly updated to use[]types.PoolI.ingest/sqs/pools/transformer/taker_fee_test.go (3)
4-4: Clean import update.The import has been correctly updated to use the new
typespackage.
25-25: Correctly updated type references in struct definitions.The type references have been properly updated to use
types.DenomPairandtypes.TakerFeeMap.Also applies to: 33-33, 35-35
40-40: Comprehensive test case updates.All test cases have been properly updated to use the new type references while maintaining the same test coverage and functionality.
Also applies to: 50-50, 52-52, 66-66, 77-77, 85-85, 103-103, 105-105, 119-119, 129-129, 136-136
ingest/sqs/pools/transformer/alloy_transmuter.go (1)
8-8: Clean import path update.The import path has been correctly updated to use the new package location for
sqscosmwasmpool.ingest/sqs/service/sqs_streaming_service_test.go (3)
11-11: LGTM! Import path updated correctly.The import path change from
sqsdomaintoingest/typesaligns with the PR objective of decoupling from SQS.
106-110: LGTM! Type references updated consistently.The variable type and function call updates from
sqsdomaintotypespackage are consistent with the import changes.
Line range hint
137-140: LGTM! Consistent type updates across test cases.The type changes from
sqsdomaintotypesare consistently applied across all test cases.Also applies to: 328-331, 685-688
ingest/sqs/pools/transformer/pool_transformer.go (4)
13-14: LGTM! Import paths updated correctly.The import paths for both
typesandsqscosmwasmpoolpackages are properly updated to use the new module structure.
123-127: LGTM! Function signature updated correctly.The
Transformmethod signature has been properly updated to use the newtypespackage while maintaining the same interface contract.
134-134: LGTM! Variable type declarations updated consistently.All variable type declarations have been properly updated to use the new
typespackage.Also applies to: 187-188, 278-279
303-305: LGTM! Struct field types updated correctly.The struct field types have been properly updated to use the new
typespackage while maintaining the same structure.proto/osmosis/ingest/v1beta1/ingest.proto (1)
5-5: Verify Osmosis version number in go_package pathPlease confirm that
v28is the correct version for the Osmosis repository. This is important for maintaining consistent versioning across the codebase.✅ Verification successful
Version number v28 in go_package path is correct
The version number
v28in the go_package path matches the module version in go.mod and is consistent with all other proto files in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the current Osmosis version in go.mod grep "osmosis-labs/osmosis/v" go.mod # Check other proto files for version consistency fd -e proto | xargs grep -l "osmosis-labs/osmosis/v"Length of output: 5096
ingest/indexer/service/indexer_streaming_service_test.go (1)
137-142: LGTM! Type changes align with decoupling objectives.The changes correctly update the type references from
sqsdomain.PoolItotypes.PoolIwhile maintaining the existing test logic and coverage.Also applies to: 328-332, 685-689
ingest/sqs/pools/transformer/pool_transformer_test.go (3)
10-11: LGTM! Import changes align with module restructuring.The imports are correctly updated to use the new
typespackage from the Osmosis repository.
47-47: LGTM! Variable type updates maintain consistency.All instances of
TakerFeeMaptype references are correctly updated fromsqsdomaintotypespackage while preserving the existing functionality.Also applies to: 67-67, 175-175, 208-208, 242-242, 277-277, 320-320, 378-378
815-815: LGTM! Function signature update preserves type safety.The
validatePoolConversionfunction signature is properly updated to usetypes.PoolIinstead ofsqsdomain.PoolI.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ingest/types/cosmwasmpool/alloy_transmuter.go (1)
20-20:⚠️ Potential issueCorrect spelling error in comment: 'Tranmuter' should be 'Transmuter'
The comment on line 20 contains a misspelling. Correcting this helps maintain code readability and adheres to coding standards.
Apply this diff to correct the spelling:
-// Tranmuter Alloyed Data, since v3.0.0 +// Transmuter Alloyed Data, since v3.0.0
🧹 Nitpick comments (4)
ingest/types/pools.go (1)
17-37: Enhance interface documentation for better clarity.While the interface is well-structured, consider adding more detailed documentation for the following methods:
GetPoolLiquidityCap(): Document the units and any constraints.GetPoolDenoms(): Clarify if the returned slice maintains any specific ordering.GetUnderlyingPool(): Explain the relationship between the wrapper and underlying pool.ingest/types/cosmwasmpool/alloy_transmuter.go (3)
49-49: Fix malformed link in commentThe link provided in the comment appears to be incorrect or malformed. Updating it ensures that future readers can access the correct documentation.
Apply this diff to fix the URL:
- // [more info](https://github.com/osmosis-labs/transmuter/tree/v3.0.0?tab=readme-ov-file#normalization-factors) + // [more info](https://github.com/osmosis-labs/transmuter/blob/v3.0.0/README.md#normalization-factors)
90-94: Consider using precise numeric types for decimal valuesFields
LatestValueandIntegralin theDivisionstruct are represented as strings. If these values are intended to be precise decimal numbers, consider usingosmomath.Decor a similar precise numeric type to enhance type safety and reduce potential parsing errors.
100-102: Consider using precise numeric types for decimal valuesIn the
ChangeLimiterstruct, theLatestValueandBoundaryOffsetfields are of type string representing decimal numbers. Using a precise numeric type likeosmomath.Deccan improve type safety and prevent potential conversion errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (15)
ingest/indexer/service/indexer_streaming_service_test.go(4 hunks)ingest/sqs/domain/ingester.go(3 hunks)ingest/sqs/domain/mocks/grpc_client_mock.go(2 hunks)ingest/sqs/domain/mocks/pool_transformer_mock.go(1 hunks)ingest/sqs/pools/transformer/export_test.go(2 hunks)ingest/sqs/pools/transformer/pool_transformer.go(5 hunks)ingest/sqs/pools/transformer/pool_transformer_test.go(11 hunks)ingest/sqs/pools/transformer/taker_fee.go(2 hunks)ingest/sqs/pools/transformer/taker_fee_test.go(8 hunks)ingest/sqs/service/grpc_client.go(3 hunks)ingest/sqs/service/sqs_streaming_service_test.go(2 hunks)ingest/types/cosmwasmpool/alloy_transmuter.go(1 hunks)ingest/types/cosmwasmpool/model.go(1 hunks)ingest/types/pools.go(1 hunks)ingest/types/taker_fee.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- ingest/sqs/domain/mocks/grpc_client_mock.go
- ingest/indexer/service/indexer_streaming_service_test.go
- ingest/sqs/pools/transformer/taker_fee.go
- ingest/sqs/service/sqs_streaming_service_test.go
- ingest/types/cosmwasmpool/model.go
- ingest/sqs/domain/ingester.go
- ingest/sqs/service/grpc_client.go
- ingest/sqs/domain/mocks/pool_transformer_mock.go
- ingest/types/taker_fee.go
- ingest/sqs/pools/transformer/export_test.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: go (02)
- GitHub Check: e2e
- GitHub Check: Summary
🔇 Additional comments (6)
ingest/types/pools.go (1)
59-122: Implementation looks good!The
PoolWrapperimplementation is clean with proper error handling and consistent denom ordering. The type assertion on line 67 ensures compile-time verification of interface implementation.ingest/sqs/pools/transformer/taker_fee_test.go (1)
4-4: Type updates look good!The changes consistently update the types from
sqsdomaintoingesttypesacross the test file while maintaining the existing test logic.Also applies to: 25-25, 33-33, 35-35, 40-40, 50-50, 52-52, 66-66, 77-77, 85-85, 103-103, 105-105, 119-119, 129-129, 136-136, 175-175, 208-208, 242-242, 277-277, 320-320, 378-378
ingest/sqs/pools/transformer/pool_transformer.go (2)
123-134: Type updates in Transform method look good!The changes correctly update the return types and map initialization to use the new
ingesttypespackage.
187-188: Type updates in ConvertPool method look good!The changes correctly update:
- Parameter and return types to use
ingesttypes.PoolI- TickModel initialization to use
ingesttypes.TickModel- PoolWrapper construction to use
ingesttypes.PoolWrapperandingesttypes.SQSPoolThe logic remains unchanged while transitioning to the new types.
Also applies to: 278-291, 303-313
ingest/sqs/pools/transformer/pool_transformer_test.go (1)
815-815: Type update in validatePoolConversion looks good!The method signature correctly updates the
actualPoolparameter type fromsqsdomain.PoolItoingesttypes.PoolI.ingest/types/cosmwasmpool/alloy_transmuter.go (1)
1-103: Overall code quality is excellentThe code is well-structured, and the implementation aligns with Go best practices. The use of descriptive naming and thorough commenting enhances readability and maintainability.
|
Addressed requested changes including including linting and spelling errors, - ready for another round! 🚀 |
PaddyMc
added
the
V:state/compatible/backport
deividaspetraitis
changed the title
* chore: decouple Node releases from SQS * chore: decouple Node releases from SQS (cherry picked from commit 8286393) # Conflicts: # go.mod * mergify/bp/v28.x/pr-8886 | Resolve conflicts --------- Co-authored-by: Deividas Petraitis <hi@deividaspetraitis.lt>
Closes: BE-658
What is the purpose of the change
Introduced changes decouples SQS deployment from Osmosis by removing
sqsdomainmodule and moving its all relevant data structures to Osmosis repo.Changes summary:
sqsdomaincontents toingest/typesingest.protointo current code generation form proto filesTesting and Verifying
This change is already covered by existing tests:
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)