Apply market hour params when starting a new round & Add min/max participants #733
Conversation
WalkthroughAdds min/max round participant counts to market-hour configuration across API, CLI, config, domain, application, and persistence layers. Updates validation, service constructor and control flow to propagate and apply these counts, including DB migrations and sqlc-generated code for Postgres and SQLite. Adjusts ban threshold type to int64. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor AdminClient
participant gRPC Handler
participant AdminService
participant Domain as Domain(MarketHour)
participant Repo as MarketHourRepo
participant DB
AdminClient->>gRPC Handler: UpdateMarketHourConfig(req)
gRPC Handler->>gRPC Handler: Parse times, durations, min/max
gRPC Handler->>AdminService: UpdateMarketHourConfig(start,end,period,interval,min,max)
AdminService->>AdminService: Validate min≤max (when both >0)
AdminService->>Domain: NewMarketHour(start,end,period,interval,min,max)
Domain-->>AdminService: MarketHour
AdminService->>Repo: Upsert(MarketHour)
Repo->>DB: UpsertMarketHour(..., round_min_participants, round_max_participants)
DB-->>Repo: OK
Repo-->>AdminService: OK
AdminService-->>gRPC Handler: OK
gRPC Handler-->>AdminClient: Success
note over AdminService,Repo: New fields min/max persisted
sequenceDiagram
autonumber
participant Service
participant MarketHourCalc as MarketHour Logic
participant Round as startRound/startConfirmation
Service->>MarketHourCalc: Get current market-hour overrides
MarketHourCalc-->>Service: rInterval, rMinCount, rMaxCount
Service->>Round: startRound(rInterval)
Service->>Round: startConfirmation(..., rMinCount, rMaxCount)
note over Round: Use provided min/max to gate participation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)internal/config/config.go (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (4)
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: 3
🧹 Nitpick comments (1)
internal/interface/grpc/handlers/adminservice.go (1)
164-180: Clarify zero semantics and validate negatives
- Document that
roundMinParticipantsCount=0means “no minimum” androundMaxParticipantsCount=0means “no maximum.”- Add an explicit check to reject negative counts (
<0) with a clear InvalidArgument error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
api-spec/protobuf/gen/ark/v1/admin.pb.gois excluded by!**/*.pb.go,!**/gen/**api-spec/protobuf/gen/ark/v1/indexer.pb.rgw.gois excluded by!**/gen/**
📒 Files selected for processing (24)
api-spec/openapi/swagger/ark/v1/admin.openapi.json(1 hunks)api-spec/protobuf/ark/v1/admin.proto(1 hunks)cmd/arkd/commands.go(3 hunks)cmd/arkd/flags.go(2 hunks)internal/config/config.go(6 hunks)internal/core/application/admin.go(3 hunks)internal/core/application/ban.go(1 hunks)internal/core/application/service.go(6 hunks)internal/core/domain/market_hour.go(1 hunks)internal/infrastructure/db/postgres/market_hour_repo.go(1 hunks)internal/infrastructure/db/postgres/migration/20251004023301_market_hour_update.down.sql(1 hunks)internal/infrastructure/db/postgres/migration/20251004023301_market_hour_update.up.sql(1 hunks)internal/infrastructure/db/postgres/sqlc/queries/db.go(1 hunks)internal/infrastructure/db/postgres/sqlc/queries/models.go(2 hunks)internal/infrastructure/db/postgres/sqlc/queries/query.sql.go(5 hunks)internal/infrastructure/db/postgres/sqlc/query.sql(1 hunks)internal/infrastructure/db/sqlite/market_hour_repo.go(1 hunks)internal/infrastructure/db/sqlite/migration/20251004023258_market_hour_update.down.sql(1 hunks)internal/infrastructure/db/sqlite/migration/20251004023258_market_hour_update.up.sql(1 hunks)internal/infrastructure/db/sqlite/sqlc/queries/db.go(1 hunks)internal/infrastructure/db/sqlite/sqlc/queries/models.go(2 hunks)internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go(5 hunks)internal/infrastructure/db/sqlite/sqlc/query.sql(1 hunks)internal/interface/grpc/handlers/adminservice.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
internal/core/domain/market_hour.go (3)
internal/config/config.go (3)
RoundInterval(147-147)RoundMinParticipantsCount(184-184)RoundMaxParticipantsCount(183-183)internal/infrastructure/db/postgres/sqlc/queries/models.go (1)
MarketHour(74-83)internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
MarketHour(72-81)
internal/infrastructure/db/sqlite/market_hour_repo.go (5)
internal/core/domain/market_hour.go (1)
MarketHour(5-13)internal/infrastructure/db/postgres/sqlc/queries/models.go (1)
MarketHour(74-83)internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
MarketHour(72-81)internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (1)
UpsertMarketHourParams(1483-1492)internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go (1)
UpsertMarketHourParams(1537-1546)
internal/core/application/service.go (2)
internal/core/application/round_report.go (1)
RoundReportService(40-50)internal/core/domain/market_hour.go (1)
NewMarketHour(15-27)
internal/infrastructure/db/postgres/market_hour_repo.go (5)
internal/core/domain/market_hour.go (1)
MarketHour(5-13)internal/infrastructure/db/postgres/sqlc/queries/models.go (1)
MarketHour(74-83)internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
MarketHour(72-81)internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (1)
UpsertMarketHourParams(1483-1492)internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go (1)
UpsertMarketHourParams(1537-1546)
internal/core/application/admin.go (2)
internal/config/config.go (2)
RoundMinParticipantsCount(184-184)RoundMaxParticipantsCount(183-183)internal/core/domain/market_hour.go (1)
NewMarketHour(15-27)
internal/config/config.go (2)
pkg/arkd-wallet-btcwallet/config/config.go (4)
Datadir(18-18)Port(17-17)LogLevel(19-19)EsploraURL(21-21)pkg/arkd-wallet/config/config.go (3)
Datadir(25-25)Port(24-24)LogLevel(26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: integration tests
- GitHub Check: unit tests
- GitHub Check: Build and Scan
🔇 Additional comments (10)
internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
72-81: LGTM!The SQLC-generated MarketHour struct correctly includes the new RoundMinParticipants and RoundMaxParticipants fields with int64 types, matching the domain model requirements.
internal/core/application/ban.go (1)
33-33: LGTM!The explicit cast to int64 ensures type consistency when comparing against
s.banThreshold, which is now int64.internal/core/domain/market_hour.go (2)
5-13: LGTM!The MarketHour domain model cleanly extends with the new participant count fields. Type choices (int64) align with persistence layers.
15-27: LGTM!The constructor signature and initialization logic correctly handle the new min/max participant count parameters.
internal/interface/grpc/handlers/adminservice.go (1)
182-185: LGTM!The service call correctly passes the new
roundMinParticipantsCountandroundMaxParticipantsCountparameters.internal/infrastructure/db/postgres/sqlc/queries/models.go (1)
74-83: LGTM!The SQLC-generated MarketHour struct correctly includes the new RoundMinParticipants and RoundMaxParticipants fields, consistent with the SQLite implementation and domain model.
internal/infrastructure/db/sqlite/market_hour_repo.go (2)
45-53: LGTM!The Get method correctly maps the database fields
RoundMinParticipantsandRoundMaxParticipantsto the domain'sRoundMinParticipantsCountandRoundMaxParticipantsCount.
57-65: LGTM!The Upsert method correctly maps the domain fields to database parameters, maintaining consistency with the Get method.
api-spec/protobuf/ark/v1/admin.proto (1)
163-164: LGTM!The MarketHourConfig message correctly extends with
round_min_participants_countandround_max_participants_countfields. Field tags 5 and 6 are sequential, and types match the domain model.api-spec/openapi/swagger/ark/v1/admin.openapi.json (1)
1038-1045: LGTM!The OpenAPI schema correctly adds
roundMaxParticipantsCountandroundMinParticipantsCountfields toMarketHourConfig, with types matching the protobuf definitions.
| if roundMinParticipantsCount > 0 { | ||
| mhConfig["roundMinParticipantsCount"] = strconv.Itoa(int(roundMinParticipantsCount)) | ||
| } | ||
| if roundMaxParticipantsCount > 0 { | ||
| mhConfig["roundMaxParticipantsCount"] = strconv.Itoa(int(roundMaxParticipantsCount)) | ||
| } |
There was a problem hiding this comment.
Allow updating participant limits back to zero
Because we only include roundMinParticipantsCount/roundMaxParticipantsCount in the payload when their value is greater than zero, an operator cannot clear a previously configured limit (the API interprets the missing field as “leave the current value as-is”). Once a non-zero limit is set, there’s no way to revert to the default/unbounded 0. Please send the value whenever the flag is explicitly provided.
- if roundMinParticipantsCount > 0 {
+ if ctx.IsSet(marketHourRoundMinParticipantsCountFlagName) {
mhConfig["roundMinParticipantsCount"] = strconv.Itoa(int(roundMinParticipantsCount))
}
- if roundMaxParticipantsCount > 0 {
+ if ctx.IsSet(marketHourRoundMaxParticipantsCountFlagName) {
mhConfig["roundMaxParticipantsCount"] = strconv.Itoa(int(roundMaxParticipantsCount))
}📝 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.
| if roundMinParticipantsCount > 0 { | |
| mhConfig["roundMinParticipantsCount"] = strconv.Itoa(int(roundMinParticipantsCount)) | |
| } | |
| if roundMaxParticipantsCount > 0 { | |
| mhConfig["roundMaxParticipantsCount"] = strconv.Itoa(int(roundMaxParticipantsCount)) | |
| } | |
| // allow explicit zero to clear previously set limits | |
| if ctx.IsSet(marketHourRoundMinParticipantsCountFlagName) { | |
| mhConfig["roundMinParticipantsCount"] = strconv.Itoa(int(roundMinParticipantsCount)) | |
| } | |
| if ctx.IsSet(marketHourRoundMaxParticipantsCountFlagName) { | |
| mhConfig["roundMaxParticipantsCount"] = strconv.Itoa(int(roundMaxParticipantsCount)) | |
| } |
🤖 Prompt for AI Agents
In cmd/arkd/commands.go around lines 601 to 606, the code only adds
roundMinParticipantsCount/roundMaxParticipantsCount to mhConfig when the value
is >0, preventing an operator from clearing a previously set limit; change the
condition to check whether the corresponding CLI flag was explicitly provided
(e.g. Flags().Changed("roundMinParticipantsCount") /
Flags().Changed("roundMaxParticipantsCount")) and, if set, always write the
value (including zero) into mhConfig using strconv.Itoa so the payload can clear
the limit when the operator supplies 0.
| MarketHourStartTime = "MARKET_HOUR_START_TIME" | ||
| MarketHourEndTime = "MARKET_HOUR_END_TIME" | ||
| MarketHourPeriod = "MARKET_HOUR_PERIOD" | ||
| MarketHourRoundInterval = "MARKET_HOUR_ROUND_INTERVAL" | ||
| MarketHourMinRoundParticipants = "MARKET_HOUR_MIN_ROUND_PARTICIPANTS_COUNT" | ||
| MarketHourMaxRoundParticipants = "MARKET_HOUR_MAX_ROUND_PARTICIPANTS_COUNT" |
There was a problem hiding this comment.
Keep existing env var keys for market-hour participant counts
Renaming MARKET_HOUR_MIN_ROUND_PARTICIPANTS / ...MAX... to include _COUNT breaks every deployment that already exports the old variable names, silently resetting the values to zero. Please keep the original key strings (or bind both old and new aliases) so existing environments continue to work.
- MarketHourMinRoundParticipants = "MARKET_HOUR_MIN_ROUND_PARTICIPANTS_COUNT"
- MarketHourMaxRoundParticipants = "MARKET_HOUR_MAX_ROUND_PARTICIPANTS_COUNT"
+ MarketHourMinRoundParticipants = "MARKET_HOUR_MIN_ROUND_PARTICIPANTS"
+ MarketHourMaxRoundParticipants = "MARKET_HOUR_MAX_ROUND_PARTICIPANTS"📝 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.
| MarketHourStartTime = "MARKET_HOUR_START_TIME" | |
| MarketHourEndTime = "MARKET_HOUR_END_TIME" | |
| MarketHourPeriod = "MARKET_HOUR_PERIOD" | |
| MarketHourRoundInterval = "MARKET_HOUR_ROUND_INTERVAL" | |
| MarketHourMinRoundParticipants = "MARKET_HOUR_MIN_ROUND_PARTICIPANTS_COUNT" | |
| MarketHourMaxRoundParticipants = "MARKET_HOUR_MAX_ROUND_PARTICIPANTS_COUNT" | |
| MarketHourStartTime = "MARKET_HOUR_START_TIME" | |
| MarketHourEndTime = "MARKET_HOUR_END_TIME" | |
| MarketHourPeriod = "MARKET_HOUR_PERIOD" | |
| MarketHourRoundInterval = "MARKET_HOUR_ROUND_INTERVAL" | |
| MarketHourMinRoundParticipants = "MARKET_HOUR_MIN_ROUND_PARTICIPANTS" | |
| MarketHourMaxRoundParticipants = "MARKET_HOUR_MAX_ROUND_PARTICIPANTS" |
🤖 Prompt for AI Agents
In internal/config/config.go around lines 175 to 180, the environment variable
keys for market-hour participant counts were changed to include a _COUNT suffix
which breaks existing deployments; restore the original key strings
MARKET_HOUR_MIN_ROUND_PARTICIPANTS and MARKET_HOUR_MAX_ROUND_PARTICIPANTS (or
accept both names) so existing envs keep working: revert the constants to the
prior names or implement lookup that checks the old key first and falls back to
the new _COUNT key, ensuring parsing and defaults remain identical.
| if roundMinParticipantsCount <= 0 { | ||
| roundMinParticipantsCount = marketHour.RoundMinParticipantsCount | ||
| } | ||
| if roundMaxParticipantsCount <= 0 { | ||
| roundMaxParticipantsCount = marketHour.RoundMaxParticipantsCount | ||
| } | ||
| if roundMaxParticipantsCount < roundMinParticipantsCount { | ||
| return fmt.Errorf( | ||
| "got round max participants %d, expected at least %d", | ||
| roundMaxParticipantsCount, roundMinParticipantsCount, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Prevent nil deref when initializing market hours
If the repo currently has no market-hour row, MarketHourRepo().Get returns nil. In that case the roundMinParticipantsCount <= 0 branch dereferences marketHour.RoundMinParticipantsCount, which will panic before we can surface a useful error. We need to require explicit counts on first creation (or only reuse stored values when marketHour != nil) to avoid the nil dereference.
- if roundMinParticipantsCount <= 0 {
- roundMinParticipantsCount = marketHour.RoundMinParticipantsCount
- }
- if roundMaxParticipantsCount <= 0 {
- roundMaxParticipantsCount = marketHour.RoundMaxParticipantsCount
- }
+ if marketHour == nil {
+ if roundMinParticipantsCount <= 0 {
+ return fmt.Errorf("missing round min participants count")
+ }
+ if roundMaxParticipantsCount <= 0 {
+ return fmt.Errorf("missing round max participants count")
+ }
+ } else {
+ if roundMinParticipantsCount <= 0 {
+ roundMinParticipantsCount = marketHour.RoundMinParticipantsCount
+ }
+ if roundMaxParticipantsCount <= 0 {
+ roundMaxParticipantsCount = marketHour.RoundMaxParticipantsCount
+ }
+ }📝 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.
| if roundMinParticipantsCount <= 0 { | |
| roundMinParticipantsCount = marketHour.RoundMinParticipantsCount | |
| } | |
| if roundMaxParticipantsCount <= 0 { | |
| roundMaxParticipantsCount = marketHour.RoundMaxParticipantsCount | |
| } | |
| if roundMaxParticipantsCount < roundMinParticipantsCount { | |
| return fmt.Errorf( | |
| "got round max participants %d, expected at least %d", | |
| roundMaxParticipantsCount, roundMinParticipantsCount, | |
| ) | |
| } | |
| // Require explicit counts on first creation, otherwise reuse stored values. | |
| if marketHour == nil { | |
| if roundMinParticipantsCount <= 0 { | |
| return fmt.Errorf("missing round min participants count") | |
| } | |
| if roundMaxParticipantsCount <= 0 { | |
| return fmt.Errorf("missing round max participants count") | |
| } | |
| } else { | |
| if roundMinParticipantsCount <= 0 { | |
| roundMinParticipantsCount = marketHour.RoundMinParticipantsCount | |
| } | |
| if roundMaxParticipantsCount <= 0 { | |
| roundMaxParticipantsCount = marketHour.RoundMaxParticipantsCount | |
| } | |
| } | |
| if roundMaxParticipantsCount < roundMinParticipantsCount { | |
| return fmt.Errorf( | |
| "got round max participants %d, expected at least %d", | |
| roundMaxParticipantsCount, roundMinParticipantsCount, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In internal/core/application/admin.go around lines 292 to 303, the code
dereferences marketHour without checking for nil which panics when no
market-hour row exists; change the logic to first check if marketHour == nil
and, if so, require the caller to provide explicit positive
roundMinParticipantsCount and roundMaxParticipantsCount (return a clear error if
they are not provided), otherwise (marketHour != nil) use
marketHour.RoundMinParticipantsCount and marketHour.RoundMaxParticipantsCount as
fallbacks when the incoming values are non-positive; finally validate that the
resolved max >= min and return the existing formatted error if not.
| repeated SweepableOutput outputs = 2; | ||
| } | ||
|
|
||
| message MarketHourConfig { |
This adds 2 new fields min/max round participants to the market hour and also adds logic to the app service to apply market hour params when starting a round within it.
Summary by CodeRabbit
New Features
Bug Fixes
Chores