fix: use loaded addresses to refresh restricted address book#3971
fix: use loaded addresses to refresh restricted address book#3971ws4charlie merged 2 commits intodevelopfrom
Conversation
📝 WalkthroughWalkthroughThe changes introduce a thread-safe method to retrieve restricted addresses and correct the loading logic to ensure addresses are sourced from the appropriate configuration file. A dedicated test validates the correct loading and retrieval of restricted addresses. The changelog is updated to document this fix. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Config as config.go
participant File as Restricted Address File
Test->>File: Write test restricted addresses to JSON file
Test->>Config: Call LoadRestrictedAddressesConfig(basePath)
Config->>File: Read zetaclient_restricted_addresses.json
File-->>Config: Return restricted addresses (JSON)
Config->>Config: Store addresses in restrictedAddressBook (lowercased)
Test->>Config: Call GetRestrictedAddresses()
Config-->>Test: Return all restricted addresses as []string
Test->>Test: Assert addresses match expected (case-insensitive)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
zetaclient/config/config.go (1)
128-135:⚠️ Potential issueGlobal map reassignment is racy – copy instead of replace
SetRestrictedAddressesFromConfigreplaces the entirerestrictedAddressBookmap without any lock.
If another goroutine is concurrently reading throughContainRestrictedAddress()(which only takes an RLock), a data race occurs because the map pointer itself changes outside the lock.Instead of swapping the map pointer, clear & repopulate the existing map under a full write-lock:
-func SetRestrictedAddressesFromConfig(cfg Config) { - restrictedAddressBook = cfg.GetRestrictedAddressBook() +func SetRestrictedAddressesFromConfig(cfg Config) { + restrictedAddressBookLock.Lock() + defer restrictedAddressBookLock.Unlock() + + // purge current entries + for k := range restrictedAddressBook { + delete(restrictedAddressBook, k) + } + + // copy from config + for k := range cfg.GetRestrictedAddressBook() { + restrictedAddressBook[strings.ToLower(k)] = true + } }This keeps the same map instance and removes the race.
Consider adding a unit test that runsSetRestrictedAddressesFromConfigin parallel withContainRestrictedAddressusing the-raceflag to confirm.
🧹 Nitpick comments (4)
zetaclient/config/config.go (1)
96-106: Pre-allocate slice & keep result deterministicMap size is already known, so allocate the slice up-front – this avoids an extra allocation and makes intent explicit.
Optionally, sorting guarantees deterministic ordering which is handy for logging and tests.- addresses := []string{} + addresses := make([]string, 0, len(restrictedAddressBook)) for addr := range restrictedAddressBook { addresses = append(addresses, addr) } + // sort.Strings(addresses) // <- optional, but recommendedzetaclient/config/config_test.go (2)
16-45: Enable test parallelisation for faster suiteThe test has no shared state with others once the temp dir is unique, so it can safely run in parallel.
-func Test_LoadRestrictedAddressesConfig(t *testing.T) { +func Test_LoadRestrictedAddressesConfig(t *testing.T) { + t.Parallel()
47-62: Mark helper ast.Helper()for cleaner stack tracesMarking the helper clarifies intent and produces nicer failure messages.
-func createRestrictedAddressesConfig(t *testing.T, basePath string, addresses []string) { +func createRestrictedAddressesConfig(t *testing.T, basePath string, addresses []string) { + t.Helper()changelog.md (1)
16-16: Update changelog entry for clarity and consistencyThe entry should adopt the imperative style and explicitly note the file being replaced. For example:
- * [3971](https://github.com/zeta-chain/node/pull/3971) - zetaclient should load restricted addresses correctly from `zetaclient_restricted_addresses.json` + * [3971](https://github.com/zeta-chain/node/pull/3971) - load restricted addresses from `zetaclient_restricted_addresses.json` instead of `zetaclient_config.json`
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md(1 hunks)zetaclient/config/config.go(2 hunks)zetaclient/config/config_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/config/config.gozetaclient/config/config_test.go
🧬 Code Graph Analysis (1)
zetaclient/config/config_test.go (2)
testutil/sample/zetaclient.go (4)
RestrictedEVMAddressTest(11-11)RestrictedBtcAddressTest(12-12)RestrictedSolAddressTest(13-13)RestrictedSuiAddressTest(14-14)zetaclient/config/config.go (2)
LoadRestrictedAddressesConfig(141-147)GetRestrictedAddresses(97-106)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-zetanode
- GitHub Check: gosec
- GitHub Check: lint
- GitHub Check: build-and-test
- GitHub Check: analyze (go)
- GitHub Check: build
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3971 +/- ##
===========================================
+ Coverage 64.13% 64.21% +0.08%
===========================================
Files 474 474
Lines 34863 34872 +9
===========================================
+ Hits 22358 22393 +35
+ Misses 11479 11448 -31
- Partials 1026 1031 +5
🚀 New features to boost your workflow:
|
3 participants
Description
The original code seems to be a typo and it still uses the old compliance config in
zetaclient_config.jsonrather than the dedicatedzetaclient_restricted_addresses.jsonfile. Newly added addresses inzetaclient_restricted_addresses.jsonwill be ignored by the zetaclients.original code:
modified code:
How Has This Been Tested?
Summary by CodeRabbit
Bug Fixes
Tests