Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds affiliate tracking configuration (AffiliateID and AffiliateBps) to THORChain providers. Configuration is sourced from environment variables and Kubernetes ConfigMaps, then propagated through provider constructors to quote requests for all supported blockchain networks. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cmd/server/main.go`:
- Around line 173-174: The config struct fields ThorChainAffiliateID and
ThorChainAffiliateBps are defined but never used; either remove these two fields
from the config in cmd/server/main.go, or if they are intended for future
plumbing, keep them but add a clear TODO comment next to ThorChainAffiliateID
and ThorChainAffiliateBps stating they must be wired into thorClient and
NewSwapSpec (or whichever component will consume them) and create a small TODO
ticket reference; ensure any kept fields are clearly marked and not left
silently unused.
In `@cmd/worker/main.go`:
- Around line 501-505: The struct tags `mapstructure:"..."` are ineffective
because config is loaded with envconfig.Process, so remove those tags from the
affected structs: update type thorChainConfig (remove `mapstructure` tags on
AffiliateID and AffiliateBps) and the verifier struct (remove `mapstructure`
tags on URL, SendToken, SwapToken, PartyPrefix) so envvar resolution relies only
on envconfig field names; keep field names intact and ensure no other
mapstructure tags remain in related config types referenced by
envconfig.Process.
In `@deploy/base/server/server.yaml`:
- Around line 101-104: The deployment currently sets THORCHAINAFFILIATEID and
THORCHAINAFFILIATEBPS (no underscores), but the server config struct expects
THORCHAIN_AFFILIATE_ID and THORCHAIN_AFFILIATE_BPS so envconfig ignores them and
defaults are used; fix by either renaming the env vars in the server deployment
to THORCHAIN_AFFILIATE_ID and THORCHAIN_AFFILIATE_BPS to match the config
struct, or (preferred) remove the hardcoded env entries and source these keys
from the existing thorchain ConfigMap the worker uses so both deployments share
the same single source of truth.
🧹 Nitpick comments (1)
internal/thorchain/provider_tron.go (1)
160-160: Pre-existing debugfmt.Printfstatements should be cleaned up.Lines 160 and 197 contain
fmt.Printfdebug statements ([TRON PROVIDER DEBUG],[TRON TX BUILDER DEBUG]) that will write to stdout in production. Consider replacing these with structured logger calls at debug level, or removing them entirely, in a follow-up. As per coding guidelines, structured logging with configurable levels should be used.
deploy/dev/01_thorchain.yaml
Outdated
| name: thorchain | ||
| data: | ||
| url: "https://thornode.ninerealms.com" | ||
| affiliate-id: "vultisig" |
There was a problem hiding this comment.
please make it vult, it needs to be short to always fit in memo
There was a problem hiding this comment.
Done, but according to the docs, we can even pass the address there
Closes vultisig/recipes#488
Summary by CodeRabbit
Release Notes
New Features
Chores