* Here's how I resolved a race condition in the encryption prompt tests:
I've refactored `promptForConfigEncryption` to accept an `io.Reader`. This allows tests to use `strings.NewReader` instead of relying on the global `os.Stdin`. This change isolates the input source for `TestPromptForConfigEncryption`, preventing concurrent access conflicts with `TestPromptForConfigKey` which uses `withInteractiveResponse` to manipulate `os.Stdin`.
The original issue manifested as a data race detected by `go test -race` between these two test functions due to their parallel execution (`t.Parallel()`) and their interaction with the shared `os.Stdin` resource.
Here are the changes I made:
- `config/config_encryption.go`:
- Modified `promptForConfigEncryption` to `promptForConfigEncryption(r io.Reader) (bool, error)`.
- Introduced `PromptForConfigEncryption()` as a public wrapper that calls the refactored function with `os.Stdin` for application use.
- `config/config.go`:
- Updated the call site for prompting config encryption to use the new `PromptForConfigEncryption()` wrapper.
- `config/config_encryption_test.go`:
- Updated `TestPromptForConfigEncryption` to call the (now unexported) `promptForConfigEncryption` with `strings.NewReader` for various input scenarios.
- `t.Parallel()` was maintained for `TestPromptForConfigEncryption`.
To verify, I confirmed that running `go test -race ./config/...` shows the previously reported race condition is no longer present. All tests in the `config` package now pass with the race detector enabled.
* Refactor(config): Remove redundant loop variable capture in test
I've removed the explicit `tc := tc` line in `TestPromptForConfigEncryption`
as it is no longer necessary for Go versions 1.22 and later.
The project's Go version (1.24.3 as per CI) handles loop variable
scoping correctly for parallel subtests, making this capture redundant.
This change is a follow-up to the fix for the race condition in issue #1928,
addressing your feedback on code style. No functional changes are introduced
by this commit.
* Style(config): Remove explicit empty string in test case
Refactors the "input_empty_eof" test case in
`TestPromptForConfigEncryption` to remove the explicit
assignment of `input: ""`. This relies on Go's default
behavior for struct field initialization (a string field
defaults to an empty string), making the code more concise.
This change is a follow-up to previous refactorings for issue #1928,
addressing your feedback on code style. No functional changes
are introduced by this commit.
* Update config/config_encryption_test.go
Co-authored-by: Ryan O'Hara-Reid <oharareid.ryan@gmail.com>
* linter: Make indenting a happy bing
* config: Rid PromptForConfigEncryption
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Ryan O'Hara-Reid <oharareid.ryan@gmail.com>
* golangci-lint: Enable usetesting and unused linters
* tests: Improve assertions in various test cases for clarity and accuracy
* tests: Enhance error assertions in TestExecuteStrategyFromFile for improved clarity
* tests: Update assertions for improved clarity and accuracy
* tests: Replace assert with require for task count checks
* config/versions/v7: Replace context.Background() with t.Context()
* Bithumb: Centralise and consoliate testPair, relax UpdateTickers check
with some glorious Doom Eternal music
* Bithumb: Use UpdatePairsOnce and update remaining pair string
* Bithumb: Add UpdatePairsOnce to TestUpdateTickers
* build/ci: Update Go to v1.24, golangci-lint to v1.64.5 and fix issues
* Address shazbert's nitters
* linter/config: Fix new linter issue and use versionSize const
* Address gk's nitters and fix additional linter issue after rebase
* Address glorious nits
* staticcheck: Fix additional linter issues after upgrading to Go 1.24.1 and golangci-lint v1.64.6
Also addresses nits
* Improve testing, assertify usage and use common.ErrParsingWSField
* TestCreateNewStrategy: Replace must > should wording
* Config: Rename DecryptConfigFile to DecryptConfigData
Because this isn't really a file, it's a byte slice
* Config: Rename EncryptConfigFile to EncryptConfigData
Because it's not actually a file
* Config: Fix config version downgrade
Fixes#1769
* Config: Version Management
* Engine: Improve visibility of TestConfigAllJsonResponse failures
* Config: Update cmd/config to allow upgrades
* Config: Add Version2 to rename GDAX
* Config: Restructure versioning to share types
This restructure allows us to share types between versions, avoids
needing to import the versions, and puts the test fixtures in same
package.
It's a win on all fronts
* Config: Fix SetNTPCheck using log
Called from engine before logger is inited, and also just wrong to use
log to communicate with user
* Config: Improve TestMigrateConfig
* Config: Drop requirement for versions to be registered in sequence
Checking the versions at Deploy is much saner.
* Config: Fix file encrypted but flag not set
* Config: Add -edit and encryption upgrade to cmd/config
This simplifies the handling for encryption prompts by moving it to a
field on config, allowing us to simplify all the places were were
passing around config
Also moves password entry to being secure (echo-off)
* Tests: Fix inconsistent should/must assertions
* test: use `T.TempDir` to create temporary test directory
This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* test: fix TestEncryptTwiceReusesSaltButNewCipher on Windows
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* test: fix TestCheckConnection on Windows
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* test: fix TestRPCServer_GetTicker_LastUpdatedNanos on Windows
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* test: cleanup TestGenerateReport
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* Bump CI versions
* Specifically set go version as 1.17.x bumps it to 1.18
* Another
* Adjust AppVeyor
* Part 1 of linter issues
* Part 2
* Fix various linters and improvements
* Part 3
* Finishing touches
* Tests and EqualFold
* Fix nitterinos plus bonus requester jobs bump for exchanges with large number of tests
* Fix nitterinos and bump golangci-lint timeout for AppVeyor
* Address nits, ensure all books are returned on err due to syncer regression
* Fix the wiggins
* Fix duplication
* Fix nitterinos
* golangci-lint/CI: Bump versions
Fix remaining linter issues
* Specifically set AppVeyor version
* Fix the infamous typos 👀
* Add go env cmd to AppVeyor
* Add go version cmd to AppVeyor
* Specify AppVeyor image, adjust linters
* Update go get to go install due to deprecation
* Bump golangci-lint timeout time for AppVeyor
* Change NW contract to NQ
* Address nitters
* GetRandomPair -> Pair{}
* Address nits
* Address time nitterinos plus additional tweaks
* More time inception upgrades!
* Bending time and space
* Speeds up tests
* Reduces time.Sleeps, lowers CreateTestBot complexity. Breaks things
* Removal of unecessary config reads. Parallel tests. Lower times
* Speeds up recent trades results
* mini update
* zoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooom
* Removes the dupes
* Lint
* post cherrypick
* Fix rare kraken data race
* Fixes banking global issues. Fixes postgres trades
* rmline for appveyor test
* Expands timeout in event that channel is closed before send
* Fix data race
* No rows, no bows and definitely no shows
* Removes parallel from createsnapshot tests
* Extends timedmutext test a smidge. Exchange fatality
* Shorter end timeframe and bigger candle
* Config: fix don't create empty dir when resolving path
* Config: refactor config file loaders
* add a layer of abstraction so that config can be loaded from non-files
* use io.Reader / io.Writer abstraction to separate data operations from
file operations
* remove dryrun option from SaveConfig - now it always saves
* rename read and save methods to mention file operations
* log error when encryption prompt fails
* as the user didn't make a choice, we'd prompt again next time the file
is loaded
* add file.Writer tests
* skip permissions test for windows
* defer creating the writer on save to the last moment
* this avoids truncating file when there is error with password prompt
* add a test
* tests with StdIn cannot run in parallel
* Config: do to not store decryption variables globally
* save session variables as unexported fields in config
* unexport internal encryption helper functions
* add encryption/decryption tests
* fix yes/no prompt returns flipped boolean
* add a test to ensure prompting for encryption works
* fix possible NPE when failing
* don't return salt if there's error
* initial update of currency system
* WIP progress
* Finish initial currency string error returns
* fix whoopsie testing for non https insecureinos
* Current WIP for getEnabledPairs check and error return
* WIP continued
* When getting enabled pairs throw error when item is not contained in available pairs list
* More updates -WIP
* Wip continued including potential interface
* Current WIP
* pairs manager pass
* drop asset string and just use the map key, plus return some errors and create more work for myself.
* clean and fixed a bug in currency.json, will not populate correctly without coinmarketcap api keys set.
* purge logger references after merge
* go mod tidy after merge
* Pointer change WIP
* fix some issues and added error returns to a few items (WIP)
* WIP
* Clean
* Fix some linter issues
* Fix more linter issues
* even more linters
* xtda nits
* revert pointer change and rm field
* Addr madcozbadd nits
* fix linter issues: shadow declarations
* Fix linter issues: gocritic huge things
* linter issue fix
* Addr nits
* flush go mod files
* after merge woops
* fix shadow dec
* Addr thrasher nits
* addr nits
* fix some issues
* more fixes
* RM println
* Addr glorious nits
* Add helper method for setting assets
* add missing format directive
* Addr nits
* Actually process new futures contracts -_- derp
* WIP for GRPC upgrade for pair management
* update config pairs
* finished disabling and enabling asset
* linked update of tradable pairs to cli
* fix oopsies
* defer writing to file on program termination for currency storage system
* update template
* don't add disabled asset items to initial sync
* Fix enable disabling a list of pairs and added in a slice error type so we can add whats allowable without throwing an error and return a report, also addressed some other nits
* WIP on getting a channel to unsub
* Wip track down unsubscribe bug and start creating streaming interface
* purge websocket orderbook object and centralised updating routine for orderbook
* general clean before interface implementation
* stage one connection interface WIP
* WIP
* repackage wshandler WIP
* find difference of subs and change signature of subscriber functions so we can batch subscriptions and unsubscription in exchanges that support it
* design change on mange subscription routine WIP
* integrated ZB with the new webosocket updates
* WIP - okex conversion
* integrate websocket upgrades for lakebtc, kraken, huobi, hitbtc, gateio, and WIP for coinbene
* integrate another range of exchanges for websocket update
* Added subscriber and unsubscriber methods to websocket functionality
* fixed tests WIP
* amalgamate cache setup with main websocket setup
* reinstate exported fields traffic alert and shutdownC to accommodate gemini and lakebtc implementations
* added in colon
* Updated websocket auth handling as they werent getting passed through. Added a setter method for websocket URL due to the Binance generated auth key/listenKey. Fixed bug which stopped reconnection.
* Fix subscribe candle bug
Fix time conversion in candle
Fix inititial candle history to datahandler
Include funding to orderbook handling
Include funding to trades
Reduce code duplication in sub and unsub functions
Added the ability to include funding currency websocket subscriptions
validated all channels and added more items todo list (Auth items)
* RM line
* bitstamp pass
* btcmarkets pass - still needs to implement unsubscriber functionality and pairs change test.
* Batch outgoing subscriptions and fix unsubscribe bug
* BTSE - bumped time to minute to reduce pinger calling by 75 calls per day. Fix authentication bug and add authentication pass into to-do. Batch outgoing subscription calls
* fix type field and batch outgoing subs and unsubs for coinbasepro
* Batch outgoing subs and unsubs
* Fixes bug when matching return from authentication
* Fix bug where params where being sent out of order due to map ,where depth items werent being subscribed too, where trying to subscribe to too many kline items caused error, where trying to get a nano secocond ID conflicted due to speed of generation.
* Add websocket capability for currency pair change by utilizing full channel subscription list in subscribe function.
* Add error handling
* Fix public: time conversions, subscription list, stopped pushing heartbeat to data handler, aggregated list of connections.
* hitBTC pass
* returned nil instead of error due to period null bids and asks updates coming through.
* Fix auth ping capture and reply. Added in interval handling for kline data. Added correct full trade data handling. Fix subscribe and unsubscribe.
* Fix when websocket auth conn and token generation fail we don't try and auth sub. Fix bug between auth and normal connection id generation and matching. Batch outgoing payloads to increase efficiency. Updated matching functions to utilise channels instead of waitgroups and go routines.
* RM debug output
* rm func to get shutdown channel
* Add unsubscriber functionality, added wsTicker type, removed return as this will impede data flow and cause reconnection when handling and processing data
* okgroup WIP
* *Added missing fields for websocket trades
*Fix bug processing kline interval
*Added fields for websocket ticker struct
*Fix auth bug
-Updated request and response matching param to interface so we can custom signature match. Stops auth subscribing before a reply is issued.
-Updated channel inclusion of pair fo auth subs as this was missing.
*Assortment of perfomance improvements
* poloniex pass
* send all trades to data handler, validated enabled and disable pairs
* initial clean
* centralised request matching mechanism
* websocket main improvements WIP
* WIP
* Websocket management via gctcli WIP
* GRPC expansion
* Updated GCTCLI with websocket url and proxy setting functionality which flushes connection
* Fix continuous spawning of routines bug on error with reconnection
* Addr linter issues
* fix subscription bug that I caused when I changed to a switch case
* fix linter issue
* fix woopsie
* End of day WIP
* Fix order submission REST, time conversion, order type conversion, orderID bugs
* fix gateio test and unsubscribe bug
* revert comment out code
* websocketAPI changed to to true in configtest.json
* fix race in gateio test
* End of day WIP for websocket tests.
* BugFix for binance when book isn't seeded. Updated websocket tests. Deprecated subscription manager. RM wrapper funcs.
* Added string title to exchange name as they are saved as lower case in type, reinstated verbose check in websocket.go
* Added verbosity check for setting websocket URL
* fix bug where the asset had a mind of its own
* purge dodgy coding
* Fix tests, drop blocking chan in websocket Dial function
* few more changes
* race condition fix for websocket tests.
* fix intermittent test failure due to underlying hash table storage
* Address madcuzbad nit
* RM superfluous printlines
* Add quick top example with paramater fields
* First pass Glorious nits
* As per madcozbad suggestion return error when enabled pair not found in full return map. Add test.
* addr madcozbadd nits
* as per glorious suggestion rm'd loadedJSON field
* adjusted ticker, added test and RM'd code that can never be executed
* Addr nits and add in crypto rand genration for ID's
* remove global channel declaration and rescoped as this was causing a lock
* as per glorious suggestion restructured return error for websocket
* addr glorious suggestions
* fix linter issues
* purge non-existent pair from testdata
* add side field to struct and parse
* addr glorious nits
* Add verbosity to error returns and logs and fix string parsing in GCTRPC
* fix speeling mistwake
* Adds websocket functionality check before flushing websocket connection
* Addr kraken panic and setting/flush websocket url stage one.
* added websocket url check before setting with tests
* Added in edge case test if by the time we call contains on available pairs it has been changed
* remove error return for func
* Continuation of tests
* continuation of tests
* Stop potential panic within pair creation function
* Implement changes to upstream
* rm sup comment
* fix bug when subscribing and unsubscribing. Also add in boolean to determine there are currencies that need to be flushed via set pairs via gctcli
* fix test
* Fix linter issues
* Fix tests
* turn websocket off in config example
* Fix issue where you cannot enable websocket when config is set to false, also added config websocket enable state saving
* Introduced err var for same error returns
* Add err var exchange base not found
* restructure function
* drop gctscript from generic response name
* drop managesub delay const as its not being used
* correctly implement websocket rate limiting for coinut
* remove quotations
* drop pair management check
* fix spelling
* return error in function to not update currency with unset role
* amalagamted enable/disable into set function and added in pairstore fetch function
* update error description
* rm function
* moved test function to sharedtestvals and move type to types.go
* append delimiter onto currency delimiter strings
* add test coverage
* rm functions as they are set as methods in base
* remove superfluous methods
* Fix issue that would occur when a subscription errored and not appending successful subs
* fix after rebase woopsie
* fix linter issues
* fix bug streamline code
* fix linter issues
* fix linter issues
* fix case where it should not change ID if set but append new
* fix whoopsie
* fix websocket tests
* fix readme, fix wrapper issues reporting template, go mod tidy
* add test coverage
* add test coverage and verified futures pariing
* add in futures bypass as its not currently supported on BTSE until API update and implementation
* removed downside/upside profit contract type as its no longer supported. Added in check in set config pairs to warn user of potential conflict and to manually remove or update.
* If asset enabled add pair and increase code coverage
* remove strings.title, set and fetch with strings.Lower but keep struct field exchangename unchanged. Streamline ticker and orderbook code.
* Add code coverage
* log error if setting default currency fails, add code coverage
* address glorious nits
* Addr xtda nits
* fix linter issues
* addr glorious nits
* xtda nits
* Addr glorious nits
* add subscription protection and removed a superfluous wait call
* fix test
* fix whoopsie
* addr xtda nits
* addr glorious nits
* Added asset types to subscriptions structs, also added in error handling for resubscription errors
* consolidated rpc returned type and added in sucessful strings
* dropped stream timing down to 100ms
* DOC changes
* proxy and url usage string additions
* WIP
* go mod tidy rides again
* Addr nits
* Addr nits, fix tests
* fix wording
* add in test case for currency matching
* Add byte length check on outbound websocket payload subscriptions
* addr thrasher nits
* Addr madcozbadd nits
* addr linter issues
* Addr glorious nits by amalgamating function into one mega amazing function.
* fix futures account subscription bug
* addr glorious nits and reinstated wg.Wait() checks
* changed string to currency delimiter string and setconnected by function
* Attempts to update orderbook so it doesn't need to sort
* Reverts the ws ob stuff. Gets rid of sorting because it happens later. Adds some exchange features
* update existing feature lists. Expands list definition to match my emotions
* Adds bithumb bitmex and bitstamp. adds a couple more types
* Features for you, features for me, features for bittrex, btcmarkets, btse, coinbasepro, coinut, exmo, gateio and gemini
* Features for hitbtc, huobi, itbit, kraken, lakebtc, lbank, localbitcoins, okcoin, okex, poloniex, yobit, zb
* Who can forget good old alphapoint?
* Adds btcmarksets websocket :glitch_crab: fixes alphapoint features
* Adds extra data not in the documentation :/
* Replaces websocket features by using protocol features. However, it breaks it due to import cycles. I'm not sure what I'll do just yet
* Removes import cycle via duplicate structs.
* Increases coverage of config with `TestCheckCurrencyConfigValues`. Moves all currency pair package types into their own files or places it at the bottom of files if necessary
* Increase coverage in code.go
* One way of determining a test has failed, is when to it fails. Removed redundant explanation
* Increases code coverage of conversion
* Lint fixes
* Fixes orderbook tests
* Re-adds sorting because its important to still have the internal pre-processed orderbook to be representative of a real orderbook
* Secret lints that did not show up via Windows linting
* Adds protocol package to contain exchange features
* Fixes protocol implementation
* Fixes ws tests
* Addresses the following: Removes st-st-stutters in config types, changes GetAvailableForexProviders -> GetSupportedForexProviders, removes errors from tests where error is nil, removes orderbook setup when not necessary, removes import newlines, removes false bools from declaration, changes should of to should have
* imports and casing
* Fixes two more nil error checks
* Fix bug where on parsing an alternate new config it will overwrite main config.json in gct dir
* Stop movement of config.json file from root dir when a new config is parsed in
* Stop overiding config.json at gct dir with new config.json from root directory
* RM LN :D
* Fix bug where promptforconfig in config_encryption.go overwrites default config
Ensure periphery command packages do not interact or save over configuration
Ensure tests to not save over or change current testdata/config
