Set referred user tier to 2 instead of 1, to preserve previous behavior:w #3200
Conversation
…#3041) Co-authored-by: anmolagrawal345 <[email protected]>
…#3042) Co-authored-by: anmolagrawal345 <[email protected]>
Co-authored-by: Justin Barnett <[email protected]>
Co-authored-by: Justin Barnett <[email protected]>
Co-authored-by: Tian <[email protected]>
… (#3138) Signed-off-by: Shrenuj Bansal <[email protected]> Co-authored-by: shrenujb <[email protected]>
…(backport #3148) (#3151) Co-authored-by: Tian <[email protected]>
…" (backport #3142) (#3152) Co-authored-by: shrenujb <[email protected]>
…backport #3140) (#3154) Co-authored-by: jyzhe <[email protected]> Co-authored-by: Tian Qin <[email protected]>
Co-authored-by: Tian <[email protected]>
Co-authored-by: Tian <[email protected]>
…) (#3175) Co-authored-by: Justin Barnett <[email protected]>
Co-authored-by: Justin Barnett <[email protected]>
) Co-authored-by: Justin Barnett <[email protected]>
…ort #3047) (#3188) Co-authored-by: Justin Barnett <[email protected]>
…te AffiliateWhitelist (Backport) (#3189)
Co-authored-by: Justin Barnett <[email protected]>
WalkthroughThis PR introduces TWAP (Time-Weighted Average Price) order support throughout the protocol and indexer, implements an affiliate system overhaul (transitioning from whitelist to overrides with 30-day volume tracking), and executes a protocol upgrade from v9.0 to v9.4 with migration logic for affiliate tiers and parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant Maker as Maker Order
participant Indexer as Indexer / Ender
participant DB as PostgreSQL
participant Fill as Fill Event
rect rgb(200, 220, 240)
Note over Maker,Fill: TWAP Order Flow
Maker->>Indexer: StatefulOrderPlacementEvent (twapOrderPlacement)
Indexer->>Indexer: Extract TWAP parameters<br/>(duration, interval, priceTolerance)
Indexer->>DB: Insert Order with TWAP fields<br/>(type=TWAP, orderFlags=128)
end
rect rgb(220, 240, 220)
Note over Maker,Fill: TWAP Suborder Fill Flow
Fill->>Indexer: OrderFillEvent (orderFlags=256)
Indexer->>Indexer: Detect TWAP_SUBORDER flag
Indexer->>Indexer: Map to parent TWAP order<br/>(adjust orderFlags & type)
Indexer->>DB: Insert/Update Fills<br/>(type=TWAP_SUBORDER)
Indexer->>DB: Update parent totalFilled<br/>(weighted average price)
end
sequenceDiagram
participant Affiliate as Affiliate Address
participant Keeper as Affiliates Keeper
participant Stats as User Stats
participant DB as KV Store
rect rgb(240, 220, 200)
Note over Affiliate,DB: Affiliate Parameter Update Flow (v9.4 Upgrade)
Keeper->>DB: Migrate tiers via UpdateAffiliateTiers
Keeper->>DB: Set AffiliateParameters<br/>(30d caps, referee tier index)
Keeper->>DB: Set AffiliateOverrides<br/>(from whitelist)
end
rect rgb(240, 240, 220)
Note over Affiliate,DB: Fee Calculation with Overrides
Affiliate->>Keeper: Query GetTakerFeeShare
Keeper->>DB: Fetch AffiliateOverridesMap
alt Affiliate in Overrides
Keeper->>Keeper: Return max tier
else Check Referred Volume
Keeper->>Stats: Fetch Affiliate_30DReferredVolumeQuoteQuantums
Keeper->>Keeper: Map volume to tier
end
Keeper->>Affiliate: Return fee share
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR spans multiple interconnected systems (TWAP feature across proto→indexer→database, affiliate system refactoring with keeper logic changes, protocol upgrade migration). While changes follow established patterns, they require careful cross-domain verification: SQL handler logic for TWAP suborder branching is dense; keeper refactoring from whitelist to overrides with 30-day stats integration is significant; upgrade migration logic must be validated. Heterogeneous changes across TypeScript, SQL, Go, and Protobuf demand separate reasoning for each area. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts (1)
1-84: Inconsistency between PR objectives and actual changes.The PR title states "Set referred user tier to 2 instead of 1, to preserve previous behavior," suggesting changes related to affiliate/referral tiers. However, this file contains only TWAP (Time-Weighted Average Price) order support implementation, with no affiliate or tier-related changes.
This suggests either:
- The PR description/title is incorrect or outdated
- The affiliate tier changes are in other files not included in this review
- Multiple unrelated changes have been combined in a single PR
Please clarify the scope of this PR. If both TWAP support and affiliate tier changes are intentional, consider:
- Updating the PR title to reflect all changes
- Splitting into separate PRs if the changes are independent
To verify if affiliate tier changes exist elsewhere in this PR:
#!/bin/bash # Search for affiliate tier or referred user changes # Look for tier-related changes rg -n -C3 "tier.*=.*2|referr.*tier|affiliate.*tier" --type=ts --type=go # Search for files modified in this PR git diff --name-only main...fix_upgradeprotocol/mocks/ClobKeeper.go (1)
984-1006: Update mock function comments to reflect new 4-argument signature.All 4 call sites have been correctly updated to the new signature:
protocol/x/clob/memclob/memclob.go:1750✓protocol/x/clob/keeper/process_operations.go:522✓protocol/x/clob/keeper/process_operations.go:644✓protocol/x/clob/keeper/process_single_match_isolated_insurance_fund_test.go:256✓However, the mock function comments are outdated and misleading:
protocol/mocks/ClobKeeper.go:983references old parameter names ("affiliatesWhitelistMap")protocol/mocks/MemClobKeeper.go:342references old parameter names ("affiliatesWhitelistMap")Update both comments to: "ProcessSingleMatch provides a mock function with given fields: ctx, matchWithOrders, affiliateOverrides, affiliateParameters"
🧹 Nitpick comments (45)
proto/dydxprotocol/stats/stats.proto (2)
19-22: Clarify units for notional comment.“USDC … in quantums” may be chain‑specific; consider “quote asset quantums” for precision. No wire change.
29-29: Nit: fix typo “occured” → “occurred”.Generated code will inherit this; change proto and re‑gen.
- // The fills that occured on this block. + // The fills that occurred on this block.indexer/packages/v4-protos/src/codegen/dydxprotocol/stats/stats.ts (1)
7-8: Nit: “occured” in comments (inherited from proto).Will be fixed by updating proto comment and re‑generating. No manual edits here.
Also applies to: 40-51, 108-112
Makefile (1)
20-26: Consider reusing the proto-clean target.The proto-gen-clean target duplicates the cleanup commands from proto-clean (lines 22-23 duplicate lines 17-18). You could invoke
make proto-cleaninstead to avoid duplication.proto-gen-clean: @echo "Cleaning old artifacts" - @rm -rf ./proto/.proto-export - @rm -rf ./.proto-export + @$(MAKE) proto-clean @echo "Generating Protobuf files" @$(protoImage) sh ./protocol/scripts/protocgen.sh @cd proto && make proto-export-v4-proto-jsindexer/packages/postgres/src/types/trade-types.ts (1)
10-12: FillType.TWAP_SUBORDER verified; adding test remains optionalVerification confirms FillType.TWAP_SUBORDER is defined in fill-types.ts (line 31) and the mapping is correctly implemented. Adding a unit test for fillTypeToTradeType(TWAP_SUBORDER) would be a good practice but is not required—no existing tests found for trade-types.
indexer/services/comlink/public/swagger.json (4)
568-570: Enum additions: FillType OFFSETTING, TWAP_SUBORDERConfirm indexer emits these values and downstream clients are updated. Otherwise, client enum validation may reject responses.
If TWAP_SUBORDER fills are experimental, consider documenting value semantics in public docs to reduce client confusion. Based on learnings.
1023-1025: Enum additions: OrderStatus UNTRIGGERED, ERRORDefine when these are returned and ensure they are included anywhere API filters by status (e.g., query params, examples).
Add short descriptions for these statuses in API docs to aid client handling.
1052-1055: Enum additions: OrderType TAKE_PROFIT_MARKET, TWAP, TWAP_SUBORDEREnsure websocket/types and DB models are aligned and that listOrders filters accept these values.
Add a brief note that TWAP_SUBORDER represents child slices, while TWAP is the parent envelope, to prevent misuse in client-side filtering.
1117-1125: New optional fields: duration, interval, priceTolerance
- Types are string; verify they’re consistently formatted (e.g., ISO-8601 duration vs number of seconds).
- Consider marking them clearly as present only for TWAP orders.
If these are numeric quantities, prefer number with units documented; otherwise, specify exact string format in the schema description.
protocol/app/upgrades/v9.3/upgrade.go (1)
12-21: Minor: pass tagged sdkCtx into migrations for consistent loggingYou unwrap for logging but call RunMigrations with the untagged ctx. Consider using sdkCtx for consistency.
- return mm.RunMigrations(ctx, configurator, vm) + return mm.RunMigrations(sdkCtx, configurator, vm)indexer/services/ender/src/scripts/helpers/dydx_uuid_from_order_id.sql (1)
8-15: TWAP suborders mapped to parent UUID — goodNice guard. One nit: if orderFlags ever contains non-numeric text, the cast can fail. Safe-guard with COALESCE to avoid accidental errors.
- IF (order_id->>'orderFlags')::bigint = constants.order_flag_twap_suborder() THEN + IF COALESCE(((order_id->>'orderFlags')::bigint), -1) = constants.order_flag_twap_suborder() THENprotocol/app/testdata/default_genesis_state.json (1)
38-43: New affiliate tier addedLGTM. One readability nit: avoid scientific notation for integers in genesis to prevent ambiguity.
- "req_staked_whole_coins": 1e+08, + "req_staked_whole_coins": 100000000,protocol/app/upgrades/v9.3/constants.go (1)
1-15: Unused upgrade metadata (v9.3).This file defines v9.3 but the app wires only v9.4. Either wire v9.3 in protocol/app/upgrades.go or remove this file to avoid dead code drift.
protocol/x/affiliates/keeper/msg_server.go (1)
77-89: Authority error type (optional).Consider returning a typed SDK error (e.g., unauthorized) instead of a plain error string for better ABCI codes and client UX.
- if !k.Keeper.HasAuthority(msg.Authority) { - return nil, errors.New("invalid authority") - } + if !k.Keeper.HasAuthority(msg.Authority) { + return nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "invalid authority") + }(Note: import the appropriate SDK errors package for your version.)
indexer/services/ender/__tests__/handlers/stateful-order/stateful-order-removal-handler.test.ts (1)
201-205: Avoid magic numbers for orderFlags in DB seeds.Use the constant instead of '128' for clarity:
- orderFlags: '128', + orderFlags: ORDER_FLAG_TWAP.toString(),Also applies to: 245-248
protocol/x/affiliates/keeper/grpc_query_test.go (4)
32-40: OK to pivot expected fields; avoid future overflow risk in test literals.The switch to
ReferredVolume_30DRollingand current zero expectations are fine. To avoid accidental uint64 overflow if tier requirements change, prefer big.Int math when constructing staked amounts in tests instead ofuint64(...) * 1e18.- StakedAmount: dtypes.NewIntFromUint64( - uint64(types.DefaultAffiliateTiers.Tiers[0].ReqStakedWholeCoins) * 1e18, - ), + StakedAmount: dtypes.SerializableInt{ + big.NewInt(0).Mul( + big.NewInt(int64(types.DefaultAffiliateTiers.Tiers[0].ReqStakedWholeCoins)), + big.NewInt(1_000_000_000_000_000_000), + ), + },
66-74: Same overflow nit as above.Mirror the big.Int construction here to keep tests consistent and future-proof.
107-112: Case name nit: uses overrides, not whitelist.This scenario sets
AffiliateOverridesrather than the legacy whitelist. Consider renaming the table key"Whitelisted"to"Overridden"for clarity; assertions themselves look correct.Also applies to: 128-132
264-280: Good coverage for parameters; please add an explicit check for referee min tier = 2.Given the PR goal (referee minimum fee tier index = 2), assert it directly so regressions are caught.
res, err := k.AffiliateParameters(ctx, req) require.NoError(t, err) require.NotNil(t, res) require.Equal(t, &types.AffiliateParametersResponse{Parameters: types.DefaultAffiliateParameters}, res) +require.Equal(t, uint32(2), res.Parameters.RefereeMinimumFeeTierIdx)indexer/packages/postgres/src/models/order-model.ts (1)
77-81: Required keys with default null: confirm Ajv defaults or populate explicitly to avoid validation failures.
duration,interval,priceTolerance, andorderRouterAddressare inrequiredbut allownull. If Ajv isn’t configured withuseDefaults, inserts missing these keys will fail validation. Either ensure defaults are applied or drop them fromrequired.- 'orderRouterAddress', - 'duration', - 'interval', - 'priceTolerance', + // Leave optional unless builder always sets them: + // 'orderRouterAddress', + // 'duration', + // 'interval', + // 'priceTolerance',indexer/services/ender/__tests__/scripts/scripts.test.ts (1)
166-180: Mapping tests for TWAP/TWAP_SUBORDER look goodCovers core flags and condition types and asserts parity with protocolTranslations. Optionally add a case with invalid/combined flags to ensure function rejects them.
indexer/packages/postgres/src/db/migrations/migration_files/20250423144330_add_twap_fields_to_orders_and_fills_table.ts (2)
6-10: Avoid using “interval” as a column name; prefer explicit ints for TWAP fields
- interval is a PostgreSQL data type keyword; using it as a column can be awkward. Prefer a clearer name like twapInterval (or twap_interval) to avoid quoting issues.
- duration, interval, priceTolerance fit safely in int4; storing as string isn’t necessary unless other layers require strings.
- await knex.schema.alterTable('orders', (table) => { - table.string('duration').nullable(); - table.string('interval').nullable(); - table.string('priceTolerance').nullable(); - }); + await knex.schema.alterTable('orders', (table) => { + table.integer('duration').nullable(); + table.integer('twapInterval').nullable(); // avoid reserved word + table.integer('priceTolerance').nullable(); + });Note: Adjust corresponding model/types and down() accordingly if you adopt this.
1-1: Knex import stylePrefer
import { Knex } from 'knex'for v2 typings overimport * as Knex from 'knex'.-import * as Knex from 'knex'; +import { Knex } from 'knex';proto/dydxprotocol/indexer/protocol/v1/clob.proto (1)
165-180: TwapParameters docs are clear; consider tiny wording tweakDocs/readability look good. You could add “seconds” explicitly to price tolerance comment or cross‑reference ppm definition elsewhere for consistency.
protocol/testutil/memclob/keeper.go (1)
276-283: Nit: swap local names to match maps for readabilityLocal var names are inverted vs. the backing maps. No functional impact, but mildly confusing.
- if statefulOrdersExpiringDirtyState, exists := f.timeToStatefulOrdersExpiring[goodTilBlockTime]; exists { - orderIds = append(orderIds, statefulOrdersExpiringDirtyState...) + if statefulOrdersExpiringState, exists := f.timeToStatefulOrdersExpiring[goodTilBlockTime]; exists { + orderIds = append(orderIds, statefulOrdersExpiringState...) } - if statefulOrdersExpiringState, exists := f.dirtyTimeToStatefulOrdersExpiring[goodTilBlockTime]; exists { - orderIds = append(orderIds, statefulOrdersExpiringState...) + if statefulOrdersExpiringDirtyState, exists := f.dirtyTimeToStatefulOrdersExpiring[goodTilBlockTime]; exists { + orderIds = append(orderIds, statefulOrdersExpiringDirtyState...) }protocol/app/upgrades/v9.4/upgrade.go (1)
41-49: Deduplicate overrides to avoid repeated addresses across tiersIf an address appears in multiple whitelist tiers, it will be duplicated. Make overrides deterministic and idempotent by de‑duping (optionally sort).
- overrides := affiliatetypes.AffiliateOverrides{} - var overridesList []string - for _, addr := range whitelist.Tiers { - overridesList = append(overridesList, addr.Addresses...) - } - overrides.Addresses = overridesList + overrides := affiliatetypes.AffiliateOverrides{} + seen := make(map[string]struct{}) + for _, tier := range whitelist.Tiers { + for _, a := range tier.Addresses { + if _, ok := seen[a]; !ok { + seen[a] = struct{}{} + overrides.Addresses = append(overrides.Addresses, a) + } + } + } + // Optional: sort.Strings(overrides.Addresses) for stable ordering.protocol/app/upgrades/v9.4/constants.go (1)
19-43: Typo: PreviousAffilliateTiers variable nameMinor rename for correctness; adjust references where used.
- PreviousAffilliateTiers = affiliatetypes.AffiliateTiers{ + PreviousAffiliateTiers = affiliatetypes.AffiliateTiers{indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts (2)
1634-1638: Fix comment math for quoteAmountThe values are correct, but the comment should reflect
1e-14 * 1e-2 = 1e-16(not1e-1 = 1e-15).- const quoteAmount: string = '0.0000000000000001'; // quote amount is price * fillAmount = 1e-14 * 1e-1 = 1e-15 + const quoteAmount: string = '0.0000000000000001'; // quote amount = price * fillAmount = 1e-14 * 1e-2 = 1e-16
1885-1892: Align status comment with valueStatus is set to FILLED; the trailing comment claims “open”. Update the comment for clarity.
- status: OrderStatus.FILLED, // orderSize > totalFilled so status is open + status: OrderStatus.FILLED,indexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql (1)
120-135: Null-safety for weighted average in TWAP suborder updates.If price/totalFilled were ever NULL (edge restore/backfill), the average may miscompute. Consider COALESCE.
- order_record."price" = dydx_get_weighted_average(order_record."price", order_record."totalFilled", maker_price, fill_amount); - order_record."totalFilled" = order_record."totalFilled" + fill_amount; + order_record."price" = dydx_get_weighted_average( + COALESCE(order_record."price", maker_price), + COALESCE(order_record."totalFilled", 0), + maker_price, + fill_amount + ); + order_record."totalFilled" = COALESCE(order_record."totalFilled", 0) + fill_amount;protocol/app/upgrades/v9.4/upgrade_container_test.go (1)
65-77: Add explicit assertion for refereeMinimumFeeTierIdx == 2 (PR objective).You already compare full parameters; a focused check improves failure messaging.
require.Equal(t, v_9_4.UpdatedAffiliateParameters, paramsResp.Parameters) + require.Equal(t, uint32(2), paramsResp.Parameters.RefereeMinimumFeeTierIdx, + "referred user minimum fee tier index should be 2 to preserve prior behavior")indexer/services/ender/src/scripts/handlers/dydx_stateful_order_handler.sql (1)
196-196: Typo: "Unkonwn" -> "Unknown".Minor cleanup for clearer errors.
- RAISE EXCEPTION 'Unkonwn sub-event type %', event_data; + RAISE EXCEPTION 'Unknown sub-event type %', event_data;protocol/x/affiliates/keeper/grpc_query.go (1)
41-51: Prefer consistent normalized address use for stakedAmountUse addr.String() (already validated/normalized) for GetStakedAmount to avoid subtle formatting mismatches.
- stakedAmount := k.statsKeeper.GetStakedAmount(ctx, req.GetAddress()) + stakedAmount := k.statsKeeper.GetStakedAmount(ctx, addr.String())indexer/services/ender/src/scripts/helpers/dydx_liquidation_fill_handler_per_order.sql (2)
133-171: TWAP suborder update path: weighted average and partial UPDATE — good; add small guardsLogic is sound. To harden:
- Use COALESCE for totalFilled in wavg in case of unexpected NULLs.
- Reuse a local order_flags variable to avoid repeated jsonb_extract_path calls.
-- before the IF: order_flags bigint := jsonb_extract_path(order_, 'orderId', 'orderFlags')::bigint; -- inside TWAP_SUBORDER branch order_record."price" = dydx_get_weighted_average( order_record."price", - order_record."totalFilled", + COALESCE(order_record."totalFilled", 0), maker_price, fill_amount ); order_record."totalFilled" = COALESCE(order_record."totalFilled", 0) + fill_amount; order_record."status" = dydx_get_order_status(order_record."totalFilled", order_record."size", 'NOT_CANCELED', order_flags, order_record."timeInForce");
184-190: Mapping suborders to parent on INSERT: confirm desired price/type semantics
- Setting type='TWAP' and price=maker_price for a first-seen suborder implies the parent’s initial price equals the first fill price. Confirm this matches API expectations for parent order price.
- Duration/interval/priceTolerance default NULLs look fine given migrations.
If parent should carry original parent-submitted price (not first fill), consider storing a separate parent price when available via event payload and use that instead of maker_price here.
Also applies to: 192-209
indexer/services/comlink/public/api-documentation.md (1)
2302-2304: Document new TWAP fields and enums for usability
- duration/interval/priceTolerance: specify units (seconds? blocks? ms?) and whether they are integer strings or decimal strings.
- OrderStatus.ERROR: add a brief meaning and common causes.
- TWAP_SUBORDER in OrderType/FillType/TradeType: clarify that suborders are internal children; how they appear in APIs; relation to parent TWAP order.
Also applies to: 2377-2379, 2419-2420, 2544-2547, 2590-2592, 2631-2633, 2641-2642, 2727-2730, 5704-5707, 4996-4997, 6125-6126
indexer/packages/v4-protos/src/codegen/dydxprotocol/affiliates/tx.ts (2)
96-99: Silence Biome’s noEmptyInterface for generated responsesThese are generated empty responses; prefer not to hand-edit shapes. Add a file-level Biome ignore for this rule to avoid CI noise.
+/* biome-ignore-file lint/suspicious/noEmptyInterface: generated file; empty response interfaces are intentional */ import { AffiliateTiers, AffiliateTiersSDKType, AffiliateWhitelist, AffiliateWhitelistSDKType, AffiliateParameters, AffiliateParametersSDKType, AffiliateOverrides, AffiliateOverridesSDKType } from "./affiliates";Alternatively, exclude codegen directories in Biome config for this rule.
Also applies to: 120-123
1-1: Optional: narrow importsIf tree-shaking matters in consumers, consider splitting imports to only used types in this file; otherwise OK for codegen.
protocol/mocks/MemClobKeeper.go (1)
342-349: Docstring still references old parameter nameComment says “affiliatesWhitelistMap” but params are affiliateOverrides, affiliateParameters. Regenerate mocks or update the comment to avoid confusion.
-// ProcessSingleMatch provides a mock function with given fields: ctx, matchWithOrders, affiliatesWhitelistMap +// ProcessSingleMatch provides a mock function with given fields: ctx, matchWithOrders, affiliateOverrides, affiliateParametersindexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (1)
818-836: DRY up conditional and TWAP branchesBoth branches build identical orderObject. Consider unifying to reduce duplication.
- if (isConditionalOrder(order)) { + if (isConditionalOrder(order) || isTwapOrder(order)) { orderObject = { ...order!, timeInForce: apiTranslations.orderTIFToAPITIF(order!.timeInForce), postOnly: apiTranslations.isOrderTIFPostOnly(order!.timeInForce), goodTilBlock: order!.goodTilBlock, goodTilBlockTime: order!.goodTilBlockTime, ticker, }; - } else if (isTwapOrder(order)) { - orderObject = { ...same as above... }; } else { // unchanged }protocol/x/affiliates/keeper/keeper_test.go (3)
590-609: Rename test to reflect overrides, not whitelist.This test exercises AffiliateOverrides, but the name suggests whitelist. Rename for clarity to avoid confusion during future maintenance.
663-671: Remove unused testcase fields.Fields referrals, expectedVolume, referreeAddressesToVerify, expectedCommissions are never asserted. Drop or assert them to keep tests lean and intention‑revealing.
1062-1067: Drop redundant error assertion in loop.require.NoError(t, err) is re-asserting a stale err and adds no signal here. Remove it.
- require.NoError(t, err) require.Equal(t, tc.expectedAttributedVolume[idx], referrerUser.Affiliate_30DReferredVolumeQuoteQuantums)protocol/mocks/ClobKeeper.go (1)
983-990: Update mock method comment to new params.Doc still says affiliatesWhitelistMap; the signature now takes affiliateOverrides and affiliateParameters. Update the comment to prevent misuse.
-// ProcessSingleMatch provides a mock function with given fields: ctx, matchWithOrders, affiliatesWhitelistMap +// ProcessSingleMatch provides a mock function with given fields: ctx, matchWithOrders, affiliateOverrides, affiliateParametersindexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/protocol/v1/clob.ts (1)
452-466: Bring order_flags docs in sync with new bits.Docs still reference “2nd and 3rd bits” and list ShortTerm=0 as a bit. With TWAP and TWAP_SUBORDER added, update the narrative to avoid confusion.
- * order_flags represent order flags for the order. This field is invalid if - * it's greater than 257. Each bit represents a different flag. - * - * The following are the valid orderId flags: - * ShortTerm = uint32(0) - * Conditional = uint32(32) - * LongTerm = uint32(64) - * Twap = uint32(128) - * TwapSuborder = uint32(256) (for internal use only) - * - * If both bits are set or bits other than the 2nd and 3rd are set, the order - * ID is invalid. + * order_flags represent order flags for the order. Flags are bitfields: + * Conditional=32, LongTerm=64, Twap=128, TwapSuborder=256 (internal only). + * ShortTerm orders have 0 flags. Combinations that conflict with protocol + * rules are invalid (e.g., mutually exclusive types set simultaneously).Also applies to: 490-504
| export function protocolConditionTypeToOrderType( | ||
| protocolConditionType: IndexerOrder_ConditionType, | ||
| orderFlag: number = 32, | ||
| ): OrderType { | ||
| if (!(protocolConditionType in CONDITION_TYPE_TO_ORDER_TYPE_MAP)) { | ||
| throw new Error(`Unexpected ConditionType: ${protocolConditionType}`); | ||
| } | ||
|
|
||
| return CONDITION_TYPE_TO_ORDER_TYPE_MAP[protocolConditionType]; | ||
| switch (orderFlag) { | ||
| case ORDER_FLAG_SHORT_TERM: | ||
| return OrderType.LIMIT; | ||
| case ORDER_FLAG_CONDITIONAL: | ||
| switch (protocolConditionType) { | ||
| case IndexerOrder_ConditionType.UNRECOGNIZED: | ||
| case IndexerOrder_ConditionType.CONDITION_TYPE_UNSPECIFIED: | ||
| return OrderType.LIMIT; | ||
| case IndexerOrder_ConditionType.CONDITION_TYPE_STOP_LOSS: | ||
| return OrderType.STOP_LIMIT; | ||
| case IndexerOrder_ConditionType.CONDITION_TYPE_TAKE_PROFIT: | ||
| return OrderType.TAKE_PROFIT; | ||
| default: | ||
| throw new Error(`Unexpected ConditionType: ${protocolConditionType}`); | ||
| } | ||
| case ORDER_FLAG_LONG_TERM: | ||
| return OrderType.LIMIT; | ||
| case ORDER_FLAG_TWAP: | ||
| return OrderType.TWAP; | ||
| case ORDER_FLAG_TWAP_SUBORDER: | ||
| return OrderType.TWAP_SUBORDER; | ||
| default: | ||
| throw new Error(`Unexpected OrderFlags: ${orderFlag}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Make orderFlag explicit; audit call sites.
Defaulting orderFlag to 32 (CONDITIONAL) can misclassify orders if callers forget to pass the flag. Prefer making it required or validating non-conditional flags.
Search for callers that omit the second param:
If any exist, thread through the real orderFlags.
🏁 Script executed:
#!/bin/bash
rg -nP 'protocolConditionTypeToOrderType\s*\([^,)]*\)' --type=ts --glob 'indexer/**/*.ts' -C2Length of output: 2804
Let me expand the search to check if production code (non-test files) also has this pattern:Now let me search for production code calls only:
🏁 Script executed:
#!/bin/bash
rg -nP 'protocolConditionTypeToOrderType\s*\([^,)]*\)' --type=ts -g '!**/test/**' -g '!**/__tests__/**' --glob 'indexer/**/*.ts' -C2Length of output: 148
Let me try a broader search approach:
🏁 Script executed:
#!/bin/bash
# Search for all calls to protocolConditionTypeToOrderType
rg -nP 'protocolConditionTypeToOrderType\s*\(' --type=ts -C2 | grep -v '__tests__\|\.test\.ts'Length of output: 1488
Let me examine the context around these production call sites to determine if they have access to orderFlag:
🏁 Script executed:
#!/bin/bash
# Check order-remove-handler.ts context
echo "=== order-remove-handler.ts context ===" && \
sed -n '680,695p' git/indexer/services/vulcan/src/handlers/order-remove-handler.ts && \
echo "" && \
echo "=== websocket-helper.ts context ===" && \
sed -n '65,85p' git/indexer/packages/kafka/src/websocket-helper.tsLength of output: 147
Let me find the correct file paths:
🏁 Script executed:
#!/bin/bash
# Find the correct paths
fd -type f 'order-remove-handler.ts' && \
fd -type f 'websocket-helper.ts'Length of output: 170
Let me fix the search syntax:
🏁 Script executed:
#!/bin/bash
# Find the correct paths using correct fd syntax
fd 'order-remove-handler.ts' && \
fd 'websocket-helper.ts'Length of output: 107
Now let me examine the context around these call sites:
🏁 Script executed:
#!/bin/bash
# Check order-remove-handler.ts context around line 689
echo "=== order-remove-handler.ts context (lines 680-700) ===" && \
sed -n '680,700p' indexer/services/vulcan/src/handlers/order-remove-handler.ts && \
echo "" && \
echo "=== websocket-helper.ts context (lines 60-85) ===" && \
sed -n '60,85p' indexer/packages/kafka/src/websocket-helper.tsLength of output: 2556
Thread actual orderFlags through call sites; don't rely on default.
Found 2 production call sites that omit the orderFlag parameter despite having access to the actual orderFlags:
indexer/services/vulcan/src/handlers/order-remove-handler.tsline 689: Should passredisOrder.order!.orderId!.orderFlagsindexer/packages/kafka/src/websocket-helper.tsline 75: Should passredisOrder.order!.orderId!.orderFlags
Both currently use the default 32 (CONDITIONAL), which can misclassify orders. Update these calls to pass the actual orderFlags value as the second argument.
🤖 Prompt for AI Agents
In indexer/packages/postgres/src/lib/protocol-translations.ts around lines 329
to 360, the protocolConditionTypeToOrderType function accepts an orderFlag but
callers are relying on its default (32) which can misclassify orders; update the
two production call sites to pass the actual orderFlags value: in
indexer/services/vulcan/src/handlers/order-remove-handler.ts at line ~689, pass
redisOrder.order!.orderId!.orderFlags as the second argument; and in
indexer/packages/kafka/src/websocket-helper.ts at line ~75, pass
redisOrder.order!.orderId!.orderFlags as the second argument; keep existing
behavior for null/undefined orderFlags only if handled elsewhere, otherwise
propagate the raw value through to this function.
| export interface AffiliateOverridesRequest {} | ||
| /** | ||
| * AffiliateOverridesRequest is the request type for the | ||
| * Query/AffiliateOverrides RPC method. | ||
| */ | ||
|
|
||
| export interface AffiliateOverridesRequestSDKType {} | ||
| /** |
There was a problem hiding this comment.
Biome: replace empty interfaces with type aliases (codegen lint fix)
Empty interfaces trigger lint/suspicious/noEmptyInterface. Use type aliases or exclude codegen from lint.
-export interface AffiliateOverridesRequest {}
+export type AffiliateOverridesRequest = {};
-export interface AffiliateOverridesRequestSDKType {}
+export type AffiliateOverridesRequestSDKType = {};
-export interface AffiliateParametersRequest {}
+export type AffiliateParametersRequest = {};
-export interface AffiliateParametersRequestSDKType {}
+export type AffiliateParametersRequestSDKType = {};Alternatively, exclude **/codegen/** from Biome lint in your config.
Also applies to: 199-206
🧰 Tools
🪛 Biome (2.1.2)
[error] 171-171: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 177-177: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🤖 Prompt for AI Agents
In indexer/packages/v4-protos/src/codegen/dydxprotocol/affiliates/query.ts
around lines 171-178 (and similarly lines 199-206), there are empty interface
declarations; replace each empty interface with a type alias (e.g., "export type
AffiliateOverridesRequest = {}" and "export type
AffiliateOverridesRequestSDKType = {}") to satisfy Biome lint rules, or
alternatively update the project Biome config to exclude /**/codegen/** from
linting—apply the same change to the other empty interfaces noted at lines
199-206.
| await expectFillInDatabase({ | ||
| subaccountId: testConstants.defaultSubaccountId, | ||
| clientId: '123', | ||
| liquidity: Liquidity.TAKER, | ||
| size: '100000000', | ||
| price: '0.00000000000001', | ||
| quoteAmount: '0.000001', | ||
| eventId, | ||
| transactionHash: defaultTxHash, | ||
| createdAt: defaultDateTime.toISO(), | ||
| createdAtHeight: defaultHeight, | ||
| type: FillType.TWAP_SUBORDER, | ||
| clobPairId: testConstants.defaultPerpetualMarket3.clobPairId, | ||
| side: protocolTranslations.protocolOrderSideToOrderSide(suborder2.side), | ||
| orderFlags: parentTwapOrder.orderId!.orderFlags.toString(), | ||
| clientMetadata: suborder1.clientMetadata.toString(), | ||
| fee: defaultTakerFee, | ||
| affiliateRevShare: defaultAffiliateRevShare, | ||
| }); |
There was a problem hiding this comment.
Use suborder1.side for the first TWAP suborder fill
The first expectation references suborder2.side; both are SIDE_BUY today, but this couples the test to data and could mask regressions.
- side: protocolTranslations.protocolOrderSideToOrderSide(suborder2.side),
+ side: protocolTranslations.protocolOrderSideToOrderSide(suborder1.side),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await expectFillInDatabase({ | |
| subaccountId: testConstants.defaultSubaccountId, | |
| clientId: '123', | |
| liquidity: Liquidity.TAKER, | |
| size: '100000000', | |
| price: '0.00000000000001', | |
| quoteAmount: '0.000001', | |
| eventId, | |
| transactionHash: defaultTxHash, | |
| createdAt: defaultDateTime.toISO(), | |
| createdAtHeight: defaultHeight, | |
| type: FillType.TWAP_SUBORDER, | |
| clobPairId: testConstants.defaultPerpetualMarket3.clobPairId, | |
| side: protocolTranslations.protocolOrderSideToOrderSide(suborder2.side), | |
| orderFlags: parentTwapOrder.orderId!.orderFlags.toString(), | |
| clientMetadata: suborder1.clientMetadata.toString(), | |
| fee: defaultTakerFee, | |
| affiliateRevShare: defaultAffiliateRevShare, | |
| }); | |
| await expectFillInDatabase({ | |
| subaccountId: testConstants.defaultSubaccountId, | |
| clientId: '123', | |
| liquidity: Liquidity.TAKER, | |
| size: '100000000', | |
| price: '0.00000000000001', | |
| quoteAmount: '0.000001', | |
| eventId, | |
| transactionHash: defaultTxHash, | |
| createdAt: defaultDateTime.toISO(), | |
| createdAtHeight: defaultHeight, | |
| type: FillType.TWAP_SUBORDER, | |
| clobPairId: testConstants.defaultPerpetualMarket3.clobPairId, | |
| side: protocolTranslations.protocolOrderSideToOrderSide(suborder1.side), | |
| orderFlags: parentTwapOrder.orderId!.orderFlags.toString(), | |
| clientMetadata: suborder1.clientMetadata.toString(), | |
| fee: defaultTakerFee, | |
| affiliateRevShare: defaultAffiliateRevShare, | |
| }); |
🤖 Prompt for AI Agents
In indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts
around lines 1907 to 1925, the test expectation for the first TWAP suborder fill
incorrectly uses suborder2.side which ties the test to current fixture values;
change that field to use suborder1.side instead (still wrapped with
protocolTranslations.protocolOrderSideToOrderSide if needed) so the expectation
matches the actual first suborder and avoids coupling to other fixtures.
| orderRouterAddress: defaultOrder.orderRouterAddress, | ||
| builderAddress: testOrder.builderCodeParams?.builderAddress ?? null, | ||
| feePpm: testOrder.builderCodeParams?.feePpm.toString() ?? null, | ||
| orderRouterAddress: testOrder.orderRouterAddress, |
There was a problem hiding this comment.
Coalesce orderRouterAddress to avoid undefined vs empty-string mismatch
DB returns empty string when not set. This expectation passes undefined for TWAP/default orders without an explicit router address. Align with other tests to avoid flakiness.
- orderRouterAddress: testOrder.orderRouterAddress,
+ orderRouterAddress: testOrder.orderRouterAddress ?? '',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| orderRouterAddress: testOrder.orderRouterAddress, | |
| orderRouterAddress: testOrder.orderRouterAddress ?? '', |
🤖 Prompt for AI Agents
In
indexer/services/ender/__tests__/handlers/stateful-order/stateful-order-placement-handler.test.ts
around line 224, the test passes undefined for orderRouterAddress which causes
mismatch with the DB returning an empty string; update the test to coalesce the
value to an empty string (e.g. use the nullish coalescing operator or
equivalent) so that when orderRouterAddress is undefined the test supplies '' to
match DB behavior and other tests.
| if (this.allOrderPlacementTypesUndefined()) { | ||
| return this.logAndThrowParseMessageError( | ||
| 'One of orderPlace, orderRemoval, conditionalOrderPlacement, conditionalOrderTriggered, ' + | ||
| 'longTermOrderPlacement must be defined in StatefulOrderEvent', | ||
| 'longTermOrderPlacement, or twapOrderPlacement must be defined in StatefulOrderEvent', | ||
| { event: this.event }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Remove value returns from void method (Biome lint)
validate() returns void but returns the value of logAndThrowParseMessageError. Drop the value return. Repeat pattern across file.
- if (this.allOrderPlacementTypesUndefined()) {
- return this.logAndThrowParseMessageError(
+ if (this.allOrderPlacementTypesUndefined()) {
+ this.logAndThrowParseMessageError(
'One of orderPlace, orderRemoval, conditionalOrderPlacement, conditionalOrderTriggered, ' +
'longTermOrderPlacement, or twapOrderPlacement must be defined in StatefulOrderEvent',
{ event: this.event },
);
}🧰 Tools
🪛 Biome (2.1.2)
[error] 34-38: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
In indexer/services/ender/src/validators/stateful-order-validator.ts around
lines 33 to 39, the validate() method is declared to return void but currently
uses "return this.logAndThrowParseMessageError(...)" which returns a value;
remove the leading "return" and just call
"this.logAndThrowParseMessageError(...)" so no value is returned. Apply the same
change to any other occurrences in this file where validate() (or other void
methods) return the result of logAndThrowParseMessageError — replace "return
<call>" with just "<call>" to satisfy the Biome lint rule.
| parser.add_argument('--max-30d-revenue', type=int, required=True, help='Maximum 30d affiliate revenue per affiliate') | ||
| args = parser.parse_args() | ||
|
|
||
| counter = 0 |
There was a problem hiding this comment.
Remove unused variable.
The counter variable is assigned but never used in the retry loop.
Apply this diff:
- counter = 0
# 3 retries for the process.
for i in range(3):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| counter = 0 | |
| # 3 retries for the process. | |
| for i in range(3): |
🧰 Tools
🪛 Ruff (0.14.1)
95-95: Local variable counter is assigned to but never used
Remove assignment to unused variable counter
(F841)
🤖 Prompt for AI Agents
In protocol/scripts/affiliates/update_affiliate_parameters.py around line 95,
remove the unused variable assignment "counter = 0" since it is never referenced
in the retry loop; simply delete that line (or any other declaration of counter)
to clean up dead code and avoid unused-variable warnings.
| "referee_minimum_fee_tier_idx": int(args.referee_min_fee_tier), | ||
| "maximum_30d_affiliate_revenue_per_affiliate_quote_quantums": int(args.max_30d_revenue), | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix indentation consistency.
This line appears to use tab characters while the rest of the file uses spaces, creating inconsistent indentation.
Ensure this line uses spaces consistently with the rest of the file (likely 12 or 16 spaces based on the nesting level).
🤖 Prompt for AI Agents
In protocol/scripts/affiliates/update_affiliate_parameters.py around line 110,
the closing brace line uses a tab character while the rest of the file uses
spaces; replace the tab with the same number of spaces used by the surrounding
block (match the file's indentation level — e.g., 12 or 16 spaces) so
indentation is consistent with the rest of the file.
| func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| affiliateOverridesBytes := store.Get([]byte(types.AffiliateOverridesKey)) | ||
|
|
||
| var affiliateOverrides types.AffiliateOverrides | ||
| if affiliateOverridesBytes == nil { | ||
| // Return empty overrides if not initialized. | ||
| return types.AffiliateOverrides{}, nil | ||
| } | ||
| err := k.cdc.Unmarshal(affiliateOverridesBytes, &affiliateOverrides) | ||
| if err != nil { | ||
| return affiliateOverrides, err | ||
| } | ||
|
|
||
| return affiliateOverrides, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Fix typo and duplication: GetAllAffilliateOverrides.
Exported name has a typo (“Affilliate”) and duplicates GetAffiliateOverrides logic. Replace with a thin, deprecated wrapper delegating to GetAffiliateOverrides to avoid two code paths.
-func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) {
- store := ctx.KVStore(k.storeKey)
- affiliateOverridesBytes := store.Get([]byte(types.AffiliateOverridesKey))
-
- var affiliateOverrides types.AffiliateOverrides
- if affiliateOverridesBytes == nil {
- // Return empty overrides if not initialized.
- return types.AffiliateOverrides{}, nil
- }
- err := k.cdc.Unmarshal(affiliateOverridesBytes, &affiliateOverrides)
- if err != nil {
- return affiliateOverrides, err
- }
-
- return affiliateOverrides, nil
-}
+// Deprecated: use GetAffiliateOverrides. Kept for backward compatibility.
+func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) {
+ return k.GetAffiliateOverrides(ctx)
+}🏁 Script executed:
#!/bin/bash
# Find any references to the misspelled method to plan migration.
rg -n 'GetAllAffilliateOverrides'Length of output: 137
I need to verify the claim that GetAffiliateOverrides exists and has the same logic, and check whether these methods are part of a public API.The web search returned results about the Keeper password management product, not the codebase. Let me search the codebase directly for the GetAffiliateOverrides method and check the interface definitions.
Good context on Cosmos SDK keeper patterns. Now let me search the codebase directly for GetAffiliateOverrides and check the keeper implementation.
Now I have good context on Cosmos SDK patterns. Let me search the codebase for GetAffiliateOverrides and also verify the typo claim.
🏁 Script executed:
#!/bin/bash
# Search for GetAffiliateOverrides method definition
rg -n 'func.*GetAffiliateOverrides' protocol/x/affiliates/keeper/Length of output: 261
🏁 Script executed:
#!/bin/bash
# Look for any query interfaces or grpc files
fd -e go protocol/x/affiliates | grep -E '(grpc_query|query\.go|msg_server)'Length of output: 389
🏁 Script executed:
#!/bin/bash
# Check if these methods appear in any interface definitions
rg -n 'GetAffiliateOverrides|GetAllAffilliateOverrides' protocol/x/affiliates/ --type go -A2 -B2Length of output: 2689
Now let me verify the implementations are identical and check if GetAllAffilliateOverrides appears in any exported interfaces:
🏁 Script executed:
#!/bin/bash
# Get the full implementation of GetAffiliateOverrides to compare
sed -n '371,383p' protocol/x/affiliates/keeper/keeper.goLength of output: 495
🏁 Script executed:
#!/bin/bash
# Search for any interface definitions or proto files that might export this method
fd -type f -name '*.go' protocol/x/affiliates | xargs grep -l 'interface\|Query' | head -20Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check if GetAllAffilliateOverrides is referenced in any way outside its definition
rg 'GetAllAffilliateOverrides' protocol/x/affiliates --type go | grep -v 'func.*GetAllAffilliateOverrides'Length of output: 0
Let me fix the search and check if this method is exposed via any interface:
🏁 Script executed:
#!/bin/bash
# Find all Go files in affiliates directory
find protocol/x/affiliates -name '*.go' | xargs grep -l 'interface'Length of output: 199
🏁 Script executed:
#!/bin/bash
# Check grpc_query.go for all query methods
cat protocol/x/affiliates/keeper/grpc_query.go | grep -E '^func.*Query' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's a query.proto file defining the query service
find protocol -name 'query.proto' | xargs grep -l 'affiliates' 2>/dev/nullLength of output: 0
Let me check the query.pb.go to confirm the method signatures in any interfaces:
🏁 Script executed:
#!/bin/bash
# Check what's in query.pb.go regarding Query service/interface
grep -n 'GetAffiliateOverrides\|GetAllAffilliateOverrides' protocol/x/affiliates/types/query.pb.go | head -10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the grpc_query.go file to see actual function definitions
grep -n 'func.*\(c context.Context' protocol/x/affiliates/keeper/grpc_query.go | head -15Length of output: 23
Fix typo and duplication: GetAllAffilliateOverrides.
The exported method has a typo ("Affilliate" with 3 L's) and contains identical logic to GetAffiliateOverrides. The method is unused internally. Refactor it as a thin, deprecated wrapper delegating to GetAffiliateOverrides to consolidate code paths while maintaining backward compatibility for external consumers.
-func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) {
- store := ctx.KVStore(k.storeKey)
- affiliateOverridesBytes := store.Get([]byte(types.AffiliateOverridesKey))
-
- var affiliateOverrides types.AffiliateOverrides
- if affiliateOverridesBytes == nil {
- // Return empty overrides if not initialized.
- return types.AffiliateOverrides{}, nil
- }
- err := k.cdc.Unmarshal(affiliateOverridesBytes, &affiliateOverrides)
- if err != nil {
- return affiliateOverrides, err
- }
-
- return affiliateOverrides, nil
-}
+// Deprecated: use GetAffiliateOverrides. Kept for backward compatibility.
+func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) {
+ return k.GetAffiliateOverrides(ctx)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) { | |
| store := ctx.KVStore(k.storeKey) | |
| affiliateOverridesBytes := store.Get([]byte(types.AffiliateOverridesKey)) | |
| var affiliateOverrides types.AffiliateOverrides | |
| if affiliateOverridesBytes == nil { | |
| // Return empty overrides if not initialized. | |
| return types.AffiliateOverrides{}, nil | |
| } | |
| err := k.cdc.Unmarshal(affiliateOverridesBytes, &affiliateOverrides) | |
| if err != nil { | |
| return affiliateOverrides, err | |
| } | |
| return affiliateOverrides, nil | |
| } | |
| // Deprecated: use GetAffiliateOverrides. Kept for backward compatibility. | |
| func (k Keeper) GetAllAffilliateOverrides(ctx sdk.Context) (types.AffiliateOverrides, error) { | |
| return k.GetAffiliateOverrides(ctx) | |
| } |
🤖 Prompt for AI Agents
In protocol/x/affiliates/keeper/keeper.go around lines 129-145, the exported
function name GetAllAffilliateOverrides has a typo and duplicates
GetAffiliateOverrides; keep the existing (misspelled) exported name for backward
compatibility but convert it into a thin deprecated wrapper that delegates to
the canonical GetAffiliateOverrides implementation: replace the function body
with a single call to return k.GetAffiliateOverrides(ctx), and add a deprecation
comment above the function (e.g., "// Deprecated: use GetAffiliateOverrides") so
callers are guided to the correct API while removing duplicated logic.
| func (k Keeper) SetAffiliateOverrides(ctx sdk.Context, overrides types.AffiliateOverrides) error { | ||
| store := ctx.KVStore(k.storeKey) | ||
| affiliateOverridesBytes, err := k.cdc.Marshal(&overrides) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| store.Set([]byte(types.AffiliateOverridesKey), affiliateOverridesBytes) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate overrides input (bech32 + duplicates).
Mirror whitelist validation: reject invalid/duplicate addresses to prevent corrupt state and hard‑to‑debug behaviors.
func (k Keeper) SetAffiliateOverrides(ctx sdk.Context, overrides types.AffiliateOverrides) error {
- store := ctx.KVStore(k.storeKey)
+ store := ctx.KVStore(k.storeKey)
+ seen := make(map[string]struct{})
+ for _, addr := range overrides.Addresses {
+ if _, err := sdk.AccAddressFromBech32(addr); err != nil {
+ return errorsmod.Wrapf(types.ErrInvalidAddress, "address override: %s", addr)
+ }
+ if _, ok := seen[addr]; ok {
+ return errorsmod.Wrapf(types.ErrDuplicateAffiliateAddressForWhitelist, "duplicate address in overrides: %s", addr)
+ }
+ seen[addr] = struct{}{}
+ }
affiliateOverridesBytes, err := k.cdc.Marshal(&overrides)
if err != nil {
return err
}
store.Set([]byte(types.AffiliateOverridesKey), affiliateOverridesBytes)
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (k Keeper) SetAffiliateOverrides(ctx sdk.Context, overrides types.AffiliateOverrides) error { | |
| store := ctx.KVStore(k.storeKey) | |
| affiliateOverridesBytes, err := k.cdc.Marshal(&overrides) | |
| if err != nil { | |
| return err | |
| } | |
| store.Set([]byte(types.AffiliateOverridesKey), affiliateOverridesBytes) | |
| return nil | |
| } | |
| func (k Keeper) SetAffiliateOverrides(ctx sdk.Context, overrides types.AffiliateOverrides) error { | |
| store := ctx.KVStore(k.storeKey) | |
| seen := make(map[string]struct{}) | |
| for _, addr := range overrides.Addresses { | |
| if _, err := sdk.AccAddressFromBech32(addr); err != nil { | |
| return errorsmod.Wrapf(types.ErrInvalidAddress, "address override: %s", addr) | |
| } | |
| if _, ok := seen[addr]; ok { | |
| return errorsmod.Wrapf(types.ErrDuplicateAffiliateAddressForWhitelist, "duplicate address in overrides: %s", addr) | |
| } | |
| seen[addr] = struct{}{} | |
| } | |
| affiliateOverridesBytes, err := k.cdc.Marshal(&overrides) | |
| if err != nil { | |
| return err | |
| } | |
| store.Set([]byte(types.AffiliateOverridesKey), affiliateOverridesBytes) | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In protocol/x/affiliates/keeper/keeper.go around lines 361 to 369, the
SetAffiliateOverrides function currently writes overrides without validating
addresses or duplicates; update it to mirror whitelist validation by iterating
overrides, validate each address is a valid bech32 account address (use the
standard sdk.AccAddressFromBech32 or existing helper), reject and return an
error on any invalid address, detect duplicate addresses (e.g., track seen
addresses in a map) and return an error if duplicates are found, and only
marshal+store the overrides after all entries pass validation.
| func (k Keeper) addReferredVolumeIfQualified( | ||
| ctx sdk.Context, | ||
| referee string, | ||
| referrer string, | ||
| volume uint64, | ||
| affiliateParams types.AffiliateParameters, | ||
| previouslyAttributedVolume map[string]uint64, | ||
| ) error { | ||
| // Get the user stats from the referee | ||
| refereeUserStats := k.statsKeeper.GetUserStats(ctx, referee) | ||
| if refereeUserStats == nil { | ||
| return errorsmod.Wrapf(types.ErrUpdatingAffiliateReferredVolume, | ||
| "referee %s, refereeUserStats is nil", referee) | ||
| } | ||
|
|
||
| previousVolume := (refereeUserStats.TakerNotional + refereeUserStats.MakerNotional + | ||
| previouslyAttributedVolume[referee]) | ||
|
|
||
| // If parameter is 0 then no limit is applied | ||
| cap := affiliateParams.Maximum_30DAttributableVolumePerReferredUserQuoteQuantums | ||
| if cap != 0 { | ||
| if previousVolume >= cap { | ||
| volume = 0 | ||
| } else if previousVolume+volume > cap { | ||
| // Remainder of the volume to get them to the cap | ||
| volume = cap - previousVolume | ||
| } | ||
| } | ||
| previouslyAttributedVolume[referee] += volume | ||
|
|
||
| // Add the volume to the referrer on their 30d rolling window | ||
| if volume > 0 { | ||
| affiliateUserStats := k.statsKeeper.GetUserStats(ctx, referrer) | ||
| if affiliateUserStats != nil { | ||
| affiliateUserStats.Affiliate_30DReferredVolumeQuoteQuantums += volume | ||
| } | ||
| k.statsKeeper.SetUserStats(ctx, referrer, affiliateUserStats) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Avoid setting nil user stats; instantiate when missing.
If affiliateUserStats is nil, SetUserStats(ctx, referrer, nil) risks a panic or a silent no‑op. Create a default struct before incrementing.
@@
// Add the volume to the referrer on their 30d rolling window
if volume > 0 {
- affiliateUserStats := k.statsKeeper.GetUserStats(ctx, referrer)
- if affiliateUserStats != nil {
- affiliateUserStats.Affiliate_30DReferredVolumeQuoteQuantums += volume
- }
- k.statsKeeper.SetUserStats(ctx, referrer, affiliateUserStats)
+ affiliateUserStats := k.statsKeeper.GetUserStats(ctx, referrer)
+ if affiliateUserStats == nil {
+ affiliateUserStats = &statstypes.UserStats{}
+ }
+ affiliateUserStats.Affiliate_30DReferredVolumeQuoteQuantums += volume
+ k.statsKeeper.SetUserStats(ctx, referrer, affiliateUserStats)
}Additionally, add the missing import:
import (
"fmt"
"math/big"
+ statstypes "github.com/dydxprotocol/v4-chain/protocol/x/stats/types"
)🤖 Prompt for AI Agents
In protocol/x/affiliates/keeper/keeper.go around lines 397 to 436, avoid calling
SetUserStats with a nil affiliateUserStats: if k.statsKeeper.GetUserStats(ctx,
referrer) returns nil, instantiate a default types.UserStats (or the concrete
stats type used) before incrementing Affiliate_30DReferredVolumeQuoteQuantums
and then call SetUserStats with that non-nil value; also add the missing import
for the stats type package (or types package) needed for the struct definition
at the top of the file.
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
ERRORorder status for improved error state visibility.Enhancements