update from upstream #13
Conversation
…er/userconfigdata
The error: UnhandledPromiseRejectionWarning: TypeError: qty.toFixed is not a function
Bumps [y18n](https://github.com/yargs/y18n) from 4.0.0 to 4.0.1. - [Release notes](https://github.com/yargs/y18n/releases) - [Changelog](https://github.com/yargs/y18n/blob/master/CHANGELOG.md) - [Commits](https://github.com/yargs/y18n/commits) Signed-off-by: dependabot[bot] <[email protected]>
If a request does not return an array, then this will cause an error. You need something like this.
Allows internal wallet transfer including c2c wallet transfers.
FIX error 404
Handling 'balanceUpdate' event
2021-04-22 WEBSOCKET New field "bc" for balance change in event "ACCOUNT_UPDATE"
Added deliveryCommissionRate functionality. Documentation here: https://binance-docs.github.io/apidocs/delivery/en/#user-commission-rate-user_data
Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.7.6 to 4.7.7. - [Release notes](https://github.com/wycats/handlebars.js/releases) - [Changelog](https://github.com/handlebars-lang/handlebars.js/blob/master/release-notes.md) - [Commits](handlebars-lang/handlebars.js@v4.7.6...v4.7.7) Signed-off-by: dependabot[bot] <[email protected]>
Bumps [lodash](https://github.com/lodash/lodash) from 4.17.19 to 4.17.21. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.19...4.17.21) Signed-off-by: dependabot[bot] <[email protected]>
chore: full refactor towards class-based await/async + TS + ESM
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
| https://t.me/nodebinanceapi | ||
|
|
||
|
|
||
| **This project is powered by** <a href="https://github.com/ccxt/ccxt"><img src="https://avatars.githubusercontent.com/u/31901609" width=4% height=4%></a> |
There was a problem hiding this comment.
This image is missing a text alternative. This is a problem for people using screen readers.
Live tests protect balance
![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
♻️ Duplicate comments (1)
tests/static-tests.mjs (1)
13-19:⚠️ Potential issueFix undefined references in debug function.
The debug function references undefined
loggerand usesutilinstead of the importedutils.const debug = function ( x ) { if ( typeof ( process.env.node_binance_api ) === 'undefined' ) { return; } - logger.log( typeof ( x ) ); - logger.log( util.inspect( x ) ); + console.log( typeof ( x ) ); + console.log( utils.inspect( x ) ); }
🧹 Nitpick comments (25)
tests/package-test/test-esm.mjs (1)
4-8: Add error handling to the async function.The current implementation doesn't handle potential exceptions that might occur during the API call. This could lead to unhandled promise rejections.
async function main() { + try { const ticker = await client.bookTickers('BTCUSDT') const res = ticker['BTCUSDT']; console.log(res) + } catch (error) { + console.error('Error fetching book tickers:', error) + } }tests/package-test/test-cjs.cjs (1)
5-8: Add error handling to the async function.The current implementation doesn't handle potential exceptions that might occur during the API call. This could lead to unhandled promise rejections.
async function main() { + try { const ticker = await client.prices('BTCUSDT') console.log(ticker) + } catch (error) { + console.error('Error fetching prices:', error) + } }examples/proxy.mts (1)
15-16: Add error handling to the async function.The current implementation doesn't handle potential exceptions from the API call. This could lead to unhandled promise rejections.
async function main () { const exchange = new Binance().options({}); exchange.httpsProxy = 'http://188.34.194.190:8911'; // // ### socksProxy ### // exchange.socksProxy = 'socks5://127.0.0.1:1080'; // // ### cors/redirection proxy ### // exchange.urlProxy = 'https://example.com/?getUrl='; // + try { const res = await exchange.futuresTime(); console.log( res ); + } catch (error) { + console.error('Error fetching futures time:', error); + } }examples/binanceStreams.js (2)
5-9: Use placeholder API credentials in example code.The current example uses empty strings for API credentials. While this may work in test mode, it's better to use clear placeholders in example code to indicate that users should replace these with their own credentials.
const binance = new Binance({ - APIKEY: '', - APISECRET: '', + APIKEY: 'your-api-key', + APISECRET: 'your-api-secret', test: true });
13-13: Add stream connection handling and cleanup.The current implementation doesn't provide any feedback about the WebSocket connection status or handle cleanup when the application terminates. This can lead to lingering connections.
-binance.tradesStream("BTCUSDT", logger); +// Store the WebSocket reference for potential cleanup +const ws = binance.tradesStream("BTCUSDT", logger); + +console.log(`Connected to BTCUSDT trades stream at ${ws}`); + +// Add cleanup on process termination +process.on('SIGINT', () => { + console.log('Closing WebSocket connections...'); + binance.terminate(); + process.exit(); +});examples/class-example.mts (1)
10-10: Add type annotation for better type safety.Since this is a TypeScript file, consider adding type annotations for better type safety and code documentation.
- const order = await binance.marketBuy("LTCUSDT", 0.1); + const order: any = await binance.marketBuy("LTCUSDT", 0.1);Or better, import and use the appropriate type from the library.
.github/workflows/js.yml (1)
17-19: Consider using a larger fetch depth for better git history access.The current fetch-depth of 2 might be insufficient for generating accurate change logs or running tools that need more git history.
- uses: actions/checkout@v4 with: - fetch-depth: 2 + fetch-depth: 0tsconfig.json (2)
88-89: Enable strict type checking for better code quality.TypeScript's strict mode is currently disabled, along with
noImplicitAny. Enabling these features would help catch potential type-related bugs early in development.- "strict": false, /* Enable all strict type-checking options. */ - "noImplicitAny": false, /* Enable error reporting for expressions and declarations with an implied 'any' type. */ + "strict": true, /* Enable all strict type-checking options. */ + // "noImplicitAny": true, /* Enable error reporting for expressions and declarations with an implied 'any' type. */Note: If enabling strict mode immediately would cause too many errors, consider creating a plan to gradually migrate to strict mode.
114-115: Add newline at end of file.The file is missing a newline character at the end of the file. Add one to satisfy linting rules and maintain consistency.
} -115 +rollup.config.js (4)
5-5: Consider removing or implementing commented TypeScript plugin.There's a commented-out TypeScript plugin import. Either implement it if TypeScript integration is needed for the build process, or remove it if it's not necessary.
-// import typescript from '@rollup/plugin-typescript';
20-21: Clarify jail path comment.The comment "node resolve generate dist/cjs/js directory" doesn't clearly explain the purpose of setting the jail path to '/src'.
- // node resolve generate dist/cjs/js directory + // Restrict file access to the src directory to prevent bundling unnecessary files jail: '/src'
26-26: Consider removing commented-out configuration.There's a commented-out dynamicRequireTargets configuration. If it's not needed, consider removing it for cleaner code.
- // dynamicRequireTargets: ["**/js/src/static_dependencies/**/*.cjs"],
40-40: Add newline at end of file.Add a newline at the end of the file to satisfy linting rules and maintain consistency.
]; +tests/static-tests.mjs (1)
228-235: Remove duplicate test nameThe test at line 228 has the same name as the test at line 219, which could cause confusion. Consider renaming one of them to be more specific.
-it( 'cancel order', async function ( ) { +it( 'spot cancel order', async function ( ) { await binance.cancel( 'LTCUSDT', '34234234' ) const url = 'https://api.binance.com/api/v3/order' assert.isTrue( interceptedUrl.startsWith(url) ) const obj = urlToObject( interceptedUrl.replace(url, '') ) assert.equal( obj.orderId, '34234234' ) assert.equal( obj.symbol, 'LTCUSDT' ) })tests/live-tests.cjs (1)
239-255: Consider removing commented codeThere are several blocks of commented-out tests. Consider either removing them or adding TODOs explaining why they're commented out and when they might be uncommented.
-// describe( 'Buy', function () { -// it( 'Attempt to buy ETH', function ( done ) { -// let quantity = 1; -// let price = 0.069; -// assert( typeof ( binance.buy( 'ETHBTC', quantity, price ) ) === 'undefined', WARN_SHOULD_BE_UNDEFINED ); -// done(); -// } ).timeout( TIMEOUT ); -// } ); - -// describe( 'Sell', function () { -// it( 'Attempt to sell ETH', function ( done ) { -// let quantity = 1; -// let price = 0.069; -// assert( typeof ( binance.sell( 'ETHBTC', quantity, price ) ) === 'undefined', WARN_SHOULD_BE_UNDEFINED ); -// done(); -// } ).timeout( TIMEOUT ); -// } );Also applies to: 291-318
README.md (4)
54-54: Add descriptive alt text for better accessibility.Consider adding alt text to the image around this line to improve screen reader compatibility for visually impaired users.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
54-54: Images should have alternate text (alt text)
null(MD045, no-alt-text)
239-239: Use “number” instead of “amount” for countable nouns.In American English, “a large number of requests” often sounds more natural than “a large amount of requests,” particularly for something countable like HTTP requests.
- You need to make a large amount of requests without getting blocked + You need to make a large number of requests without getting blocked🧰 Tools
🪛 LanguageTool
[uncategorized] ~239-~239: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...our location - You need to make a large amount of requests without getting blocked - ....(AMOUNTOF_TO_NUMBEROF)
254-254: Use “a URL” instead of “an URL.”The indefinite article should be “a” when followed by a consonant sound.
- This property prepends an url to API requests. It might be useful for ... + This property prepends a url to API requests. It might be useful for ...🧰 Tools
🪛 LanguageTool
[misspelling] ~254-~254: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ... #### proxyUrl This property prepends an url to API requests. It might be useful...(EN_A_VS_AN)
2170-2170: Add alt text to the sponsor image.Providing alt text will ensure the sponsor image is equally accessible.
src/types.ts (1)
125-161: Consider marking some fields optional.Interfaces like
IConstructorArgscontain many fields that users might not always provide, such ashttpsProxyorsocksProxy. Consider making these optional if they are not strictly required.package.json (1)
33-33: Lint script targets only thesrc/directory.If you keep other directories (like
tests/ordeprecated/) that contain code requiring lint checks, consider expanding the lint target. Otherwise, this is fine.deprecated/node-binance-api.d.ts (4)
1-4: Proper file header with JSDoc-like comments.These doc comments help track authorship and references but consider adding more descriptive file-level details if you want to clarify that this file is deprecated and is only for backward compatibility.
7-27: Robust type definitions for_callback,_symbol, and_interval.Great approach, but
_callbackis typed as(...args: any) => any, which might obscure more specific callback patterns. If you know typical callback signatures, refine this to enhance type safety.- type _callback = (...args: any) => any; + type _callback = (error?: Error, data?: any) => void;
205-234:IConstructorArgscaptures margin, futures, and advanced configurations.Looks solid—especially the nested
urls. Just ensurePartial<T>usage is correct and that default fallback logic is handled in the actual code. Some properties (likelocalAddress,family) might benefit from clearer doc comments describing their behavior.
236-1588: ClassBinance: Comprehensive type declarations but high complexity.Impressive coverage of spot, margin, futures, and delivery methods. A few suggestions:
- Method Overloads: Many methods have both typed signatures and
(...args: any)expansions. Overly broad fallback might lead to silent type mismatches.- Naming Consistency: Some methods (like
mgBuywith aside: 'BUY' | 'SELL') can confuse usage. Evaluate if you want or need asideparam for a function namedmgBuy.- Return Types: Many methods return
Promise<any>. If you know the shape of returned data, specify it more precisely to help consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.github/workflows/js.yml(1 hunks).gitignore(2 hunks).mocharc.json(1 hunks).npmignore(1 hunks)README.md(20 hunks)deprecated/node-binance-api.d.ts(1 hunks)examples/binanceStreams.js(1 hunks)examples/class-example.mts(1 hunks)examples/proxy.mts(1 hunks)package.json(1 hunks)rollup.config.js(1 hunks)src/.eslintrc.json(1 hunks)src/types.ts(1 hunks)tests/binance-class-live.test.ts(1 hunks)tests/binance-class-static.test.ts(1 hunks)tests/binance-ws-futures.test.ts(1 hunks)tests/binance-ws-spot.test.ts(1 hunks)tests/cjs-test.cjs(1 hunks)tests/live-tests.cjs(1 hunks)tests/package-test/test-cjs.cjs(1 hunks)tests/package-test/test-esm.mjs(1 hunks)tests/package.sh(1 hunks)tests/static-tests.mjs(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/.eslintrc.json
- .npmignore
- .mocharc.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- tests/cjs-test.cjs
🧰 Additional context used
🧬 Code Definitions (6)
examples/binanceStreams.js (1)
src/node-binance-api.ts (1)
Binance(28-6016)
tests/binance-class-static.test.ts (1)
src/node-binance-api.ts (1)
Binance(28-6016)
tests/binance-class-live.test.ts (1)
src/node-binance-api.ts (9)
Binance(28-6016)depth(3399-3402)openOrders(888-891)orderStatus(874-880)trades(3631-3634)prevDay(3445-3448)first(3200-3202)last(3209-3211)slice(3219-3221)
tests/binance-ws-futures.test.ts (1)
src/node-binance-api.ts (5)
Binance(28-6016)log(154-156)candlesticks(3765-3769)chart(5835-5900)trades(3631-3634)
tests/binance-ws-spot.test.ts (1)
src/node-binance-api.ts (5)
Binance(28-6016)candlesticks(3765-3769)chart(5835-5900)depth(3399-3402)trades(3631-3634)
deprecated/node-binance-api.d.ts (1)
deprecated/node-binance-api.js (34)
reconnect(4983-4985)reconnect(5010-5012)reconnect(5029-5031)reconnect(5048-5050)reconnect(5067-5069)reconnect(5086-5088)reconnect(5103-5105)reconnect(5167-5169)reconnect(5230-5232)reconnect(5257-5259)reconnect(5276-5278)reconnect(5295-5297)reconnect(5314-5316)reconnect(5333-5335)reconnect(5350-5352)reconnect(5414-5416)symbol(1072-1084)symbol(1776-1788)symbol(2530-2530)symbol(4992-4992)symbol(5120-5120)symbol(5151-5151)symbol(5176-5176)symbol(5239-5239)symbol(5367-5367)symbol(5398-5398)symbol(5423-5423)symbol(5650-5650)symbol(5685-5685)symbol(5712-5712)symbol(5751-5751)symbol(5817-5817)Binance(12-12)Binance(109-109)
🪛 Shellcheck (0.10.0)
tests/package.sh
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 Gitleaks (8.21.2)
tests/binance-class-live.test.ts
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/binance-ws-futures.test.ts
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
23-23: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
README.md
[style] ~69-~69: In American English, abbreviations like “etc.” require a period.
Context: ...rary - Overridable hostnames (.us, .jp, etc) - Verbose mode to debug http requests/...
(ETC_PERIOD)
[uncategorized] ~239-~239: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...our location - You need to make a large amount of requests without getting blocked - ....
(AMOUNTOF_TO_NUMBEROF)
[grammar] ~246-~246: Uncountable nouns are usually not used with an indefinite article. Use simply “access”.
Context: ...roxy for your scripts, you need to have an access to a remote http or https proxy, so cal...
(A_UNCOUNTABLE)
[misspelling] ~254-~254: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ... #### proxyUrl This property prepends an url to API requests. It might be useful...
(EN_A_VS_AN)
[style] ~1611-~1611: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mbol+" orders:", orders); }); #### Get dust logjavascript binance.dustLog(...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~2008-~2008: Possible missing comma found.
Context: ...()usebinance.mgSell(). For market orders use binance.mgMarketBuy()andbinanc...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~2014-~2014: Possible missing preposition found.
Context: ...e 'regular account' counterparts. #### Get your Trade History for the Margin accou...
(AI_HYDRA_LEO_MISSING_TO)
[style] ~2123-~2123: In American English, abbreviations like “etc.” require a period.
Context: ...nfo: Pull minimum order size, quantity, etc](https://github.com/jaggedsoft/node-bin...
(ETC_PERIOD)
🪛 markdownlint-cli2 (0.17.2)
README.md
41-41: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
43-43: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
51-51: Bare URL used
null
(MD034, no-bare-urls)
54-54: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
101-101: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
290-290: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
356-356: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
403-403: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
1957-1957: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
2076-2076: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🪛 YAMLlint (1.35.1)
.github/workflows/js.yml
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (45)
.github/workflows/js.yml (2)
35-36: Add timeouts and environment setup for tests.Consider adding timeouts and environment variables to better control test execution and prevent long-running tests from blocking the workflow.
- name: Live Tests (TS ESM) run: npm run ts-test-live + timeout-minutes: 10 + env: + NODE_ENV: test
1-43: Missing newline at end of file.The file is missing a newline character at the end. Add one to satisfy linting rules and maintain consistency.
- name: Package test run: npm run package-test +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
tests/package.sh (3)
1-2: LGTM: Correct shebang for bash scriptThe shebang line is correctly set to use bash, which is appropriate for this script.
8-12: LGTM: Proper test execution and cleanupThe script correctly executes both ESM and CommonJS tests, captures their return codes, and performs thorough cleanup of temporary files.
14-20: LGTM: Proper exit code handlingThe script correctly handles the exit codes from both test executions and returns an appropriate status code.
tests/binance-class-static.test.ts (10)
6-9: Use environment variables for API credentials.Hardcoded API credentials pose a security risk. Consider using environment variables instead.
const binance = new Binance({ - APIKEY: 'XXXXXXXXXXXXXXXXXXXXXXX', - APISECRET: 'YYYYYYYYYYYYYYYYYYYYYY', + APIKEY: process.env.BINANCE_API_KEY, + APISECRET: process.env.BINANCE_API_SECRET, })
12-16: LGTM: Well-implemented URL parameter conversionThe
urlToObjectutility function is well-implemented, using standard JavaScript APIs to convert query parameters to an object.
23-48: LGTM: Comprehensive test setup with proper interceptionThe
beforeEachhook correctly sets up HTTP interceptors for all HTTP methods, capturing both the URL and request body for assertions in the tests.
50-53: LGTM: Well-structured API endpoint testsThese tests effectively verify that the Binance API client correctly formats the URLs for various API endpoints, including ticker prices, order books, candlesticks, trades, and position risk endpoints.
Also applies to: 55-59, 61-65, 67-71, 73-77, 79-83, 85-89, 91-95, 97-101
103-109: LGTM: Proper cancel order testsThe tests for canceling orders correctly verify both the URL format and the required parameters.
Also applies to: 111-117
119-163: LGTM: Comprehensive spot order testsThese tests thoroughly validate the behavior of spot market and limit orders, ensuring that all parameters are correctly passed in the request.
174-218: LGTM: Well-structured futures order testsThe tests for futures market and limit orders effectively verify both the URL format and the required parameters, including the custom client order ID prefix.
220-227: LGTM: Additional order testsThese tests cover additional order scenarios such as test orders and cancel operations.
Also applies to: 229-238
240-250: LGTM: Custom order ID testThis test correctly verifies that custom client order IDs can be specified and are properly passed in the request.
252-267: LGTM: Delivery order testsThese tests effectively verify the behavior of delivery market orders and order book operations.
tests/static-tests.mjs (8)
8-11: Use environment variables for API credentials.Hardcoded API credentials pose a security risk. Consider using environment variables instead.
const binance = new Binance({ - APIKEY: 'XXXXXXXXXXXXXXXXXXXXXXX', - APISECRET: 'YYYYYYYYYYYYYYYYYYYYYY', + APIKEY: process.env.BINANCE_API_KEY, + APISECRET: process.env.BINANCE_API_SECRET, })
21-25: LGTM: Well-implemented URL parameter conversionThe
urlToObjectutility function is well-implemented, using standard JavaScript APIs to convert query parameters to an object.
32-57: LGTM: Comprehensive test setup with proper interceptionThe
beforeEachhook correctly sets up HTTP interceptors for all HTTP methods, capturing both the URL and request body for assertions in the tests.
59-118: LGTM: Well-structured API endpoint testsThese tests effectively verify that the Binance API client correctly formats the URLs for various API endpoints, including ticker prices, order books, candlesticks, trades, and position risk endpoints.
127-171: LGTM: Comprehensive spot order testsThese tests thoroughly validate the behavior of spot market and limit orders, ensuring that all parameters are correctly passed in the request.
173-217: LGTM: Well-structured futures order testsThe tests for futures market and limit orders effectively verify both the URL format and the required parameters, including the custom client order ID prefix.
219-235: LGTM: Order cancellation testsThese tests ensure that order cancellation requests are properly formatted for both futures and spot markets.
237-251: LGTM: Delivery order testsThese tests effectively verify the behavior of delivery market orders and order book operations.
tests/live-tests.cjs (6)
1-8: LGTM: Proper file header with license informationThe file includes a proper header with copyright and license information.
11-18: LGTM: Well-defined constants for test warningsThe constants for test warnings are well-defined and provide clear error messages.
70-115: LGTM: Well-structured Binance object construction testsThe tests for constructing the Binance object are well-structured and cover various initialization patterns.
126-142: LGTM: Comprehensive price checking testsThese tests thoroughly validate the behavior of the prices API for both single symbol and all symbols.
257-272: LGTM: Market order testsThe tests for market buy and sell operations are well-implemented with proper assertions.
274-289: LGTM: Futures market order testsThe tests for futures market buy and sell operations are well-implemented using the dedicated futures client.
tests/binance-class-live.test.ts (2)
575-576: Verify object comparison logic in the reverse test.
The test usestoString()to compare objects, which may be unreliable and could fail depending on howtoStringis implemented.Would you like to replace this with a deeper structural comparison, such as
assert.deepEqual(...)? For instance:- assert(binance.reverse({ '3': 3, '2': 2, '1': 1 }).toString() === { '1': 1, '2': 2, '3': 3 }.toString(), 'should be {\'1\': 1, \'2\': 2, \'3\': 3 }'); + assert.deepEqual( + binance.reverse({ '3': 3, '2': 2, '1': 1 }), + { '1': 1, '2': 2, '3': 3 }, + 'should reverse the object keys properly' + );
582-586: Confirm handling of non-numeric object keys in array conversion.
expectedincludesNaNfor the keys'a', 'b', 'c'; ensure this behavior is intentional. If not, adjust the logic to properly handle non-numeric keys.Would you like to revise the
binance.array(...)method or test expectations?README.md (2)
2008-2008: Add missing comma after "For market orders."This concern was raised previously. Tagging as duplicate.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~2008-~2008: Possible missing comma found.
Context: ...()usebinance.mgSell(). For market orders usebinance.mgMarketBuy()andbinanc...(AI_HYDRA_LEO_MISSING_COMMA)
2012-2012: Add missing preposition “for” before “the 'regular account' counterparts.”This concern was also addressed in past comments.
package.json (10)
41-41: Remove extraneous script entry if accidental.
"rollup-plugin-execute": "^1.1.1"under"scripts"is unusual. Typically, plugin versions belong indevDependencieswhile script entries are short commands. Double-check if this was intentional.
45-45: Repository URL updated successfully.No issues spotted; the repository points to the new GitHub location. This looks correct.
49-51: Keywords updated.Adding “trading” and “crypto” can increase discoverability. Looks good.
53-53: Author field changed.Check that the email/domain is correct and that it meets your publishing requirements.
56-56: Bugs URL updated.Now references GitHub issues in the same repository. This is consistent with the new repo.
58-58: Homepage field updated.The homepage points to the same new repository. No concerns here.
60-92: Updated devDependencies for TypeScript, ESLint, Rollup, and testing.Large version jumps to ESLint (
^8.57.0) and Mocha (^11.1.0) may break older configs.jestwas also introduced. Confirm your test framework references are consistent (Mocha vs. Jest) and that dual usage is intentional.
3-12:✅ Verification successful
Confirm new module exports and TypeScript configuration.
Good job adding
"typings": "./dist/node-binance-api.d.ts"and setting"type": "module"with the custom"exports"field. This setup ensures consumers can import both ESM and CJS builds. Verify that any code references to the old default entry point, if there were any, are updated to align with these new fields.Please run this shell script to locate imports of
node-binance-apiin your code and confirm they align with the updated module structure:
🏁 Script executed:
#!/bin/bash rg -A 5 "from 'node-binance-api'" . rg -A 5 "require('node-binance-api')" .Length of output: 736
Module exports and TS configuration verified.
The updatedpackage.jsonnow correctly defines"typings", sets"type": "module", and provides custom"exports"for both ESM (import) and CJS (require). Our grep results confirm that consumers (e.g., in./tests/package-test/test-esm.mjsand theREADME.md) are using ESM imports (import Binance from 'node-binance-api'), and there are no legacyrequirereferences detected. Please double-check any potential legacy references manually if applicable, but overall the updated module structure appears correctly integrated.
37-40:❓ Verification inconclusive
Verify coverage scripts remain functional.
You have scripts to generate code coverage (coveralls, codacy, codecov), but these rely on Mocha+Istanbul integration. Confirm that each coverage flow is still supported under Mocha 11.1.0 and that Istanbul or NYC usage is properly configured for ESM modules.
🏁 Script executed:
#!/bin/bash # Checking references to coverage libraries: rg "istanbul|nyc" . rg "coveralls|codacy|codecov" .Length of output: 10867
Coverage Flows Verification for Mocha 11.1.0 and ESM Modules
- The package still includes the legacy Istanbul setup (
"cover": "istanbul cover _mocha --report lcovonly") alongside NYC (v17.1.0). Note that Istanbul (v0.4.5) is deprecated and may struggle with ESM module instrumentation.- Please verify that the Istanbul-based coverage command works as expected with Mocha 11.1.0 when ESM modules are in use.
- Additionally, confirm that NYC’s configuration properly supports ESM modules and that the generated lcov report is fully compatible with Coveralls, Codacy, and Codecov.
24-31:✅ Verification successful
Check that all test scripts are aligned with Mocha 11.1.0.
Mocha 11 introduced changes such as ESM-first file loading, parallel test execution, and updated CLI flags. Ensure that your test commands and any existing configuration files don’t rely on removed features (like
--compilers) or older Node.js versions.
🏁 Script executed:
#!/bin/bash # Description: Verify that no deprecated Mocha flags are being used. rg -A 3 "mocha" .Length of output: 5865
Review Approval: Mocha Test Scripts Verified
The test commands in
package.jsonhave been verified against Mocha 11.1.0. No deprecated features (such as the removed--compilersflag) are in use, and all scripts appear to be aligned with the updated Mocha behavior. The configuration in bothpackage.jsonandpackage-lock.jsonconfirms the proper version is referenced.deprecated/node-binance-api.d.ts (2)
29-203: ExtensiveIWebsocketsinterface definitions.These definitions are quite comprehensive. Ensure that all relevant parameters—like
execution_callback,subscribed_callback, etc.—match your implementation’s actual usage. If any method is only partially supported, clarifying it in doc comments can help avoid confusion.
1590-1592: Module export is a default.Exporting
Binanceas a default is consistent with yourtypingsentry inpackage.json. Confirm you want only the default export (and no named exports) for maximum backward compatibility.
|
|
||
| async function main () { | ||
| const exchange = new Binance().options({}); | ||
| exchange.httpsProxy = 'http://188.34.194.190:8911'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded IP address with a placeholder in example code.
Using a hardcoded IP address in example code is risky as it may become unavailable or unintentionally abused if users copy the example directly. Consider using a placeholder or example domain.
- exchange.httpsProxy = 'http://188.34.194.190:8911';
+ exchange.httpsProxy = 'http://your-proxy-server:port';📝 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.
| exchange.httpsProxy = 'http://188.34.194.190:8911'; | |
| exchange.httpsProxy = 'http://your-proxy-server:port'; |
| const binance = new Binance({ | ||
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | ||
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', | ||
| test: true | ||
| }); |
There was a problem hiding this comment.
Remove hardcoded API credentials.
The example contains hardcoded API credentials which should not be included in code, even if they're for testing purposes. This poses a security risk if accidentally committed to version control.
- const binance = new Binance({
- APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ',
- APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i',
- test: true
- });
+ const binance = new Binance({
+ APIKEY: '<your_api_key>',
+ APISECRET: '<your_api_secret>',
+ test: true
+ });📝 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.
| const binance = new Binance({ | |
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | |
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', | |
| test: true | |
| }); | |
| const binance = new Binance({ | |
| APIKEY: '<your_api_key>', | |
| APISECRET: '<your_api_secret>', | |
| test: true | |
| }); |
| async function main() { | ||
| const binance = new Binance({ | ||
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | ||
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', | ||
| test: true | ||
| }); | ||
| const order = await binance.marketBuy("LTCUSDT", 0.1); | ||
| console.log( order ); | ||
| console.log( 'Hello, World!' ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for API calls.
The market buy operation could fail for various reasons (network issues, insufficient funds, etc.), but there's no error handling to gracefully handle failures.
async function main() {
const binance = new Binance({
APIKEY: '<your_api_key>',
APISECRET: '<your_api_secret>',
test: true
});
- const order = await binance.marketBuy("LTCUSDT", 0.1);
- console.log( order );
- console.log( 'Hello, World!' );
+ try {
+ const order = await binance.marketBuy("LTCUSDT", 0.1);
+ console.log( order );
+ console.log( 'Hello, World!' );
+ } catch (error) {
+ console.error('Error executing market buy:', error);
+ }
}📝 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.
| async function main() { | |
| const binance = new Binance({ | |
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | |
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', | |
| test: true | |
| }); | |
| const order = await binance.marketBuy("LTCUSDT", 0.1); | |
| console.log( order ); | |
| console.log( 'Hello, World!' ); | |
| } | |
| async function main() { | |
| const binance = new Binance({ | |
| APIKEY: '<your_api_key>', | |
| APISECRET: '<your_api_secret>', | |
| test: true | |
| }); | |
| try { | |
| const order = await binance.marketBuy("LTCUSDT", 0.1); | |
| console.log( order ); | |
| console.log( 'Hello, World!' ); | |
| } catch (error) { | |
| console.error('Error executing market buy:', error); | |
| } | |
| } |
| npm pack . --silent | ||
| mv node-binance-api*.tgz ./tests/package-test/ | ||
| cd ./tests/package-test | ||
| npm init -y > /dev/null | ||
| npm install node-binance-api*.tgz |
There was a problem hiding this comment.
Add error handling for directory change operation
The cd command on line 5 should include error handling to prevent the script from continuing execution in the wrong directory if the change directory operation fails.
npm pack . --silent
mv node-binance-api*.tgz ./tests/package-test/
-cd ./tests/package-test
+cd ./tests/package-test || exit 1
npm init -y > /dev/null
npm install node-binance-api*.tgz📝 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.
| npm pack . --silent | |
| mv node-binance-api*.tgz ./tests/package-test/ | |
| cd ./tests/package-test | |
| npm init -y > /dev/null | |
| npm install node-binance-api*.tgz | |
| npm pack . --silent | |
| mv node-binance-api*.tgz ./tests/package-test/ | |
| cd ./tests/package-test || exit 1 | |
| npm init -y > /dev/null | |
| npm install node-binance-api*.tgz |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 5-5: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| let debug = function ( x ) { | ||
| if ( typeof ( process.env.node_binance_api ) === 'undefined' ) { | ||
| return; | ||
| } | ||
| logger.log( typeof ( x ) ); | ||
| logger.log( util.inspect( x ) ); | ||
| } |
There was a problem hiding this comment.
Fix undefined reference in debug function
Similar to the static-tests.mjs file, the debug function references an undefined logger variable.
let debug = function ( x ) {
if ( typeof ( process.env.node_binance_api ) === 'undefined' ) {
return;
}
- logger.log( typeof ( x ) );
- logger.log( util.inspect( x ) );
+ console.log( typeof ( x ) );
+ console.log( util.inspect( x ) );
}📝 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.
| let debug = function ( x ) { | |
| if ( typeof ( process.env.node_binance_api ) === 'undefined' ) { | |
| return; | |
| } | |
| logger.log( typeof ( x ) ); | |
| logger.log( util.inspect( x ) ); | |
| } | |
| let debug = function ( x ) { | |
| if ( typeof ( process.env.node_binance_api ) === 'undefined' ) { | |
| return; | |
| } | |
| console.log( typeof ( x ) ); | |
| console.log( util.inspect( x ) ); | |
| } |
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | ||
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', |
There was a problem hiding this comment.
Remove or mask exposed API keys for spot trading.
Storing API keys in plain text can lead to unauthorized usage if the repo or logs are compromised. Use environment variables or a secure secrets manager instead.
For example:
- APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ',
- APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i',
+ APIKEY: process.env.TEST_SPOT_APIKEY || '',
+ APISECRET: process.env.TEST_SPOT_APISECRET || '',📝 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.
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | |
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', | |
| APIKEY: process.env.TEST_SPOT_APIKEY || '', | |
| APISECRET: process.env.TEST_SPOT_APISECRET || '', |
| APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', | ||
| APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', |
There was a problem hiding this comment.
Remove or mask exposed API keys.
Leaking API keys, even test ones, poses a serious security risk. Consider using environment variables or a dedicated secret storage system.
Illustrative patch:
- APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ',
- APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i',
...
- APIKEY: '227719da8d8499e8d3461587d19f259c0b39c2b462a77c9b748a6119abd74401',
- APISECRET: 'b14b935f9cfacc5dec829008733c40da0588051f29a44625c34967b45c11d73c',
+ APIKEY: process.env.SPOT_TEST_APIKEY || '',
+ APISECRET: process.env.SPOT_TEST_APISECRET || '',
...
+ APIKEY: process.env.FUTURES_TEST_APIKEY || '',
+ APISECRET: process.env.FUTURES_TEST_APISECRET || '',Also applies to: 22-23
| assert( Object.prototype.hasOwnProperty.call( candlesticks, 's' ), WARN_SHOULD_HAVE_KEY + 'k' ); | ||
|
|
There was a problem hiding this comment.
Fix the key mismatch in the candlesticks assertion.
Line 61 attempts to check property 's' but claims it is 'k'. Update the assertion to reference 'k' consistently.
Proposed fix:
- assert( Object.prototype.hasOwnProperty.call( candlesticks, 's' ), WARN_SHOULD_HAVE_KEY + 'k' );
+ assert( Object.prototype.hasOwnProperty.call( candlesticks, 'k' ), WARN_SHOULD_HAVE_KEY + 'k' );📝 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.
| assert( Object.prototype.hasOwnProperty.call( candlesticks, 's' ), WARN_SHOULD_HAVE_KEY + 'k' ); | |
| assert( Object.prototype.hasOwnProperty.call( candlesticks, 'k' ), WARN_SHOULD_HAVE_KEY + 'k' ); |
| export interface CancelOrder { | ||
| symbol: string | ||
| origClientOrderId: string | ||
| orderId: number | ||
| orderListId: number | ||
| clientOrderId: string | ||
| price: string | ||
| origQty: string | ||
| executedQty: string | ||
| cummulativeQuoteQty: string | ||
| status: string | ||
| timeInForce: string | ||
| type: OrderType | ||
| side: OrderSide | ||
| } |
There was a problem hiding this comment.
Duplicate interface “CancelOrder.”
There is a second CancelOrder interface at lines 321–335 that appears identical to the interface at lines 234–248. This duplication may cause confusion or overshadowing. Consider refactoring to use a single CancelOrder interface.
- export interface CancelOrder {
- symbol: string
- origClientOrderId: string
- orderId: number
- orderListId: number
- clientOrderId: string
- price: string
- origQty: string
- executedQty: string
- cummulativeQuoteQty: string
- status: string
- timeInForce: string
- type: OrderType
- side: OrderSide
- }📝 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.
| export interface CancelOrder { | |
| symbol: string | |
| origClientOrderId: string | |
| orderId: number | |
| orderListId: number | |
| clientOrderId: string | |
| price: string | |
| origQty: string | |
| executedQty: string | |
| cummulativeQuoteQty: string | |
| status: string | |
| timeInForce: string | |
| type: OrderType | |
| side: OrderSide | |
| } |
| "https-proxy-agent": "^7.0.0", | ||
| "json-bigint": "^1.0.0", | ||
| "lodash.zipobject": "^4.1.3", | ||
| "node-fetch": "^3.3.2", | ||
| "socks-proxy-agent": "^8.0.5", | ||
| "string-hash": "^1.1.3", | ||
| "url": "^0.11.0", | ||
| "ws": "^4.1.0" | ||
| "ws": "^7.2.0" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate dependency upgrades for potential breaking changes.
Dependencies like https-proxy-agent and socks-proxy-agent have significant version jumps. [email protected] in particular has a new constructor signature. If your code previously used the old signature, you'll need to update it to separate connection details and agent options.
🏁 Script executed:
#!/bin/bash
# Description: Identify existing usage of SocksProxyAgent to confirm the new signature is used.
# This requires the repository to be cloned to parse the code.
rg -A 3 "SocksProxyAgent\\(" .Length of output: 4140
Update usage of SocksProxyAgent to conform with the new constructor signature
The dependency upgrade to [email protected] changes the constructor signature. While most instances in the codebase have been updated to pass an options object (i.e. using an object with separate connection details), one occurrence in src/node-binance-api.ts still calls:
- reqOptions.agent = new SocksProxyAgent(socksproxy);
+ reqOptions.agent = new SocksProxyAgent({
+ protocol: this.parseProxy(socksproxy)[0],
+ host: this.parseProxy(socksproxy)[1],
+ port: this.parseProxy(socksproxy)[2]
+ });Please update this instance so that all usages use the new signature, ensuring compatibility with the upgraded dependency.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/class-example.mts (1)
5-9:⚠️ Potential issueRemove hardcoded API credentials.
The example contains hardcoded API credentials which should not be included in code, even if they're for testing purposes. This poses a security risk if accidentally committed to version control.
const binance = new Binance({ - APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', - APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', + APIKEY: '<your_api_key>', + APISECRET: '<your_api_secret>', test: true });🧰 Tools
🪛 Gitleaks (8.21.2)
6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🧹 Nitpick comments (3)
examples/class-example.mts (2)
15-15: Add top-level error handling.The main function is called without any error handling, which could cause unhandled promise rejections.
-main() +main().catch(error => { + console.error('Unhandled error in main function:', error); + process.exit(1); +});
1-15: Include comments to explain the example's purpose.This example lacks explanatory comments that would help users understand its purpose and how to use it effectively.
import Binance from "../src/node-binance-api.js" +/** + * This example demonstrates how to use the Binance class to execute a market buy order. + * Make sure to replace the API credentials with your own and set test: true for testing. + */ async function main() { const binance = new Binance({ APIKEY: '<your_api_key>', APISECRET: '<your_api_secret>', test: true }); + // Example: Execute a market buy order for 0.1 LTC using USDT const order = await binance.marketBuy("LTCUSDT", 0.1); console.log( order ); console.log( 'Hello, World!' ); }🧰 Tools
🪛 Gitleaks (8.21.2)
6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/binance-class-live.test.ts (1)
214-214: Correct the assertion message to match the expected key count.
The code asserts thatObject.keys(depth).lengthis 4, but the failure message says "should have length 3."Proposed adjustment:
- assert(Object.keys(depth).length === 4, 'should have length 3'); + assert(Object.keys(depth).length === 4, 'should have length 4');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/class-example.mts(1 hunks)examples/proxy.mts(1 hunks)tests/binance-class-live.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/proxy.mts
🧰 Additional context used
🪛 Gitleaks (8.21.2)
tests/binance-class-live.test.ts
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
examples/class-example.mts
6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (4)
examples/class-example.mts (1)
10-12: Add error handling for API calls.The market buy operation could fail for various reasons (network issues, insufficient funds, etc.), but there's no error handling to gracefully handle failures.
- const order = await binance.marketBuy("LTCUSDT", 0.1); - console.log( order ); - console.log( 'Hello, World!' ); + try { + const order = await binance.marketBuy("LTCUSDT", 0.1); + console.log( order ); + console.log( 'Hello, World!' ); + } catch (error) { + console.error('Error executing market buy:', error); + }tests/binance-class-live.test.ts (3)
38-39: Remove or mask exposed API keys (spot).
These keys (lines 38-39) are still in plaintext, potentially allowing malicious users to misuse spot trading if the repo or logs are compromised. Use environment variables or a secure secrets manager instead.Example fix:
- APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', - APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', + APIKEY: process.env.TEST_SPOT_APIKEY || '', + APISECRET: process.env.TEST_SPOT_APISECRET || '',🧰 Tools
🪛 Gitleaks (8.21.2)
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
44-45: Remove or mask exposed API keys (futures).
Lines 44-45 contain plaintext futures API keys. Consider storing them in environment variables or a secure vault for added protection.Example fix:
- APIKEY: '227719da8d8499e8d3461587d19f259c0b39c2b462a77c9b748a6119abd74401', - APISECRET: 'b14b935f9cfacc5dec829008733c40da0588051f29a44625c34967b45c11d73c', + APIKEY: process.env.FUTURES_TEST_APIKEY || '', + APISECRET: process.env.FUTURES_TEST_APISECRET || '',🧰 Tools
🪛 Gitleaks (8.21.2)
44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
57-58: Remove or mask exposed API keys (spot).
Refrain from hardcoding sensitive credentials in test code. Use dynamic configuration for safer handling.Example fix:
- APIKEY: 'X4BHNSimXOK6RKs2FcKqExquJtHjMxz5hWqF0BBeVnfa5bKFMk7X0wtkfEz0cPrJ', - APISECRET: 'x8gLihunpNq0d46F2q0TWJmeCDahX5LMXSlv3lSFNbMI3rujSOpTDKdhbcmPSf2i', + APIKEY: process.env.TEST_SPOT_APIKEY || '', + APISECRET: process.env.TEST_SPOT_APISECRET || '',🧰 Tools
🪛 Gitleaks (8.21.2)
57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
feat: add RSA and ed22519 support
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
tests/crypto.test.ts (2)
7-11: Consider using test key fixtures from external files.The test includes hard-coded private keys directly in the source code, which could pose a security risk if these were actual private keys instead of test fixtures.
Consider moving the test keys to separate fixture files that could be loaded during testing:
- "private_key": "-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEIPQmzwVKJETqVd7L9E/DFbkvrOigy1tLL+9QF0mSn6dV\n-----END PRIVATE KEY-----\n", + "private_key": await readFile('./tests/fixtures/ed25519_test_key.pem', 'utf8'),🧰 Tools
🪛 Gitleaks (8.21.2)
8-8: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
38-46: Add more test cases for error handling.The test suite verifies successful signature generation but doesn't test error cases such as invalid keys or incorrect passwords.
Add test cases for error scenarios:
it('should throw error for invalid private key', function() { const binance = new Binance({ APISECRET: "-----BEGIN PRIVATE KEY-----\nINVALIDKEY\n-----END PRIVATE KEY-----\n", }); assert.throws(() => { binance.generateSignature(dataQuery, false); }, /Invalid private key/); }); it('should throw error for incorrect password', function() { const binance = new Binance({ APISECRET: testCases[2].private_key, PRIVATEKEYPASSWORD: "wrongpassword", }); assert.throws(() => { binance.generateSignature(dataQuery, false); }, /bad decrypt/); });README.md (11)
2-17: Badge Section Cleanup and Clarity
The old badge section (lines 2–17) has been commented out and replaced with updated badge data. To help maintain clarity and reduce clutter in the README, consider removing these outdated commented blocks entirely once you’ve confirmed that the new badges (if any) fully meet your documentation needs.
28-39: Navigation Links Block Added
The centered navigation block between lines 28–39 provides a quick overview of the API sections. The links are visually distinct thanks to the inline styles. Please double‐check that all target anchors (e.g., “#binance-futures-api”, “#futures-websocket-streams”, etc.) match the section headings further down the document.
82-88: ESM Example is Clear
The “Getting started (ESM)” example (lines 82–88) is concise and demonstrates the basic usage of the API using async/await. Consider adding a brief comment about error handling if the example is intended for beginners.
102-234: Futures API Code Examples – Comprehensive but Consider Adding Error Handling
The series of code examples for Futures operations (starting at “#### Futures Prices” through to the various futures endpoint calls on line 234) are detailed and useful. They clearly demonstrate how to invoke methods like limit buy/sell, market orders, placing multiple orders, and even the reduceOnly option. For improved robustness, consider adding brief error-handling examples or notes in some of these callbacks to illustrate how users might handle failures (if not already detailed elsewhere in the documentation).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
102-102: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
236-263: Proxy Support Examples are Useful
The new proxy support section (lines 236–263) with examples for setting an HTTPS proxy, a proxied URL, and a socks proxy is a valuable addition for users behind restrictions. A minor nitpick: there are some LanguageTool suggestions regarding punctuation and article usage in the accompanying text (e.g. “an access” should be “access”); you may choose to update these when possible—but given your previous note on styling, this is optional.🧰 Tools
🪛 LanguageTool
[uncategorized] ~238-~238: A comma might be missing here.
Context: ...`` ### Proxy support In some specific cases using a proxy is required, for example:...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~240-~240: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...our location - You need to make a large amount of requests without getting blocked - ....(AMOUNTOF_TO_NUMBEROF)
[grammar] ~247-~247: Uncountable nouns are usually not used with an indefinite article. Use simply “access”.
Context: ...roxy for your scripts, you need to have an access to a remote http or https proxy, so cal...(A_UNCOUNTABLE)
[misspelling] ~255-~255: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ... #### proxyUrl This property prepends an url to API requests. It might be useful...(EN_A_VS_AN)
289-352: Futures WebSocket Streams – Extensive and Informative
The collection of WebSocket stream examples (lines 289–352) covers a wide range of use cases—from miniTickers to order book tickers and active subscriptions. Overall, these examples are comprehensive. In longer examples (especially those with many console.info calls), consider eventually including guidance on error handling or performance considerations when dealing with high-frequency data streams.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
291-291: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
1443-1570: Order Placement and Management Examples
The code examples for placing LIMIT, MARKET, STOP LOSS, and ICEBERG orders (lines 1443–1570) are very instructive. The inclusion of callback examples and chaining of orders (e.g. placing a market buy then following with a limit sell) provides clarity. One suggestion would be to include a note or comment on validating order parameters (e.g. quantity and price format) to help avoid common pitfalls when interacting with the live API.🧰 Tools
🪛 LanguageTool
[uncategorized] ~1557-~1557: You might be missing the article “a” here.
Context: ...bol+")", openOrders); });#### Get list of all open ordersjavascript binanc...(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 markdownlint-cli2 (0.17.2)
1469-1469: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
1500-1500: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
1638-1650: Candlestick Data Retrieval Example
The candlesticks example (lines 1638–1650) neatly shows how to retrieve historical candlestick data and extract the last close value. A small suggestion: consider adding an inline comment explaining the destructuring of the last tick for users who might be less familiar with array unpacking in JavaScript.🧰 Tools
🪛 LanguageTool
[style] ~1639-~1639: Consider using a more polite wording.
Context: ...l candlesticks, these are useful if you want to import data from earlier back in tim...(IF_YOU_WANT)
1657-1667: Complete WebSocket Chart Cache Example
The chart cache example (lines 1657–1667) is very comprehensive; it demonstrates retrieving the entire chart and then converting it to an array (with a commented-out alternative). This is very useful for developers who need historical plus live data. A note regarding potential performance issues with large datasets might be useful here.
1957-2081: Margin and Lending API Examples – Extensive and Detailed
The new sections for Binance Margin (lines 1957–2074) and Lending APIs (lines 2077–2081) are major improvements. They include examples for transfers, borrowing, repaying, placing margin orders, as well as retrieving account and lending details. These examples provide a solid foundation for developers. A suggestion would be to incorporate additional error-handling guidance or comments noting the differences between margin and spot trading (if not covered elsewhere).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1958-1958: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
2077-2077: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
2136-2167: Troubleshooting and Options – Helpful Debugging Guidelines
The “Troubleshooting” section (lines 2136–2167) includes practical examples showing how to set options such as useServerTime, recvWindow, and verbose logging. These examples will greatly assist users in debugging their connections and API calls. If you have room, a brief explanation of how each option affects API behavior could provide additional clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~2137-~2137: A comma might be missing here.
Context: ...system time is correct. If you have any suggestions don't hesitate to file an issue. Havin...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
src/types.ts (1)
125-129: Avoid storing secrets directly in code.
HavingPRIVATEKEYandPRIVATEKEYPASSWORDas interface fields may encourage placing sensitive data (like RSA/EDCSA keys) in plain text. Consider using environment variables or a secrets manager to minimize exposure risk.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/js.yml(1 hunks)README.md(20 hunks)package.json(1 hunks)src/types.ts(1 hunks)tests/crypto.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/crypto.test.ts (1)
src/node-binance-api.ts (1)
Binance(29-6061)
🪛 Gitleaks (8.21.2)
tests/crypto.test.ts
8-8: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
14-14: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
20-20: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
26-26: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🪛 LanguageTool
README.md
[style] ~69-~69: In American English, abbreviations like “etc.” require a period.
Context: ...rary - Overridable hostnames (.us, .jp, etc) - Verbose mode to debug http requests/...
(ETC_PERIOD)
[uncategorized] ~238-~238: A comma might be missing here.
Context: ...`` ### Proxy support In some specific cases using a proxy is required, for example:...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~240-~240: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...our location - You need to make a large amount of requests without getting blocked - ....
(AMOUNTOF_TO_NUMBEROF)
[grammar] ~247-~247: Uncountable nouns are usually not used with an indefinite article. Use simply “access”.
Context: ...roxy for your scripts, you need to have an access to a remote http or https proxy, so cal...
(A_UNCOUNTABLE)
[misspelling] ~255-~255: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ... #### proxyUrl This property prepends an url to API requests. It might be useful...
(EN_A_VS_AN)
[uncategorized] ~1557-~1557: You might be missing the article “a” here.
Context: ...bol+")", openOrders); }); #### Get list of all open ordersjavascript binanc...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[style] ~1612-~1612: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mbol+" orders:", orders); }); #### Get dust logjavascript binance.dustLog(...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~2124-~2124: In American English, abbreviations like “etc.” require a period.
Context: ...nfo: Pull minimum order size, quantity, etc](https://github.com/jaggedsoft/node-bin...
(ETC_PERIOD)
[uncategorized] ~2180-~2180: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...w) and give us a PR with your bugfix or improvement after. We love it ❤️ 
🪛 markdownlint-cli2 (0.17.2)
README.md
41-41: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
43-43: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
51-51: Bare URL used
null
(MD034, no-bare-urls)
54-54: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
102-102: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
291-291: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
357-357: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
404-404: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
1958-1958: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
2077-2077: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🪛 YAMLlint (1.35.1)
.github/workflows/js.yml
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (16)
.github/workflows/js.yml (2)
1-45: LGTM: Comprehensive CI workflow implementation.This GitHub Actions workflow provides excellent test coverage across various aspects of the codebase (static tests, live tests, WebSocket tests, package tests) and properly sets up the environment with caching for efficient runs.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
45-45: Add a newline at the end of the file.The file is missing a newline at the end, which is flagged by the YAML linter.
- name: Package test run: npm run package-test +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
tests/crypto.test.ts (1)
1-50: Well-structured test suite for signature verification.This test suite effectively verifies the cryptographic signature generation functionality for both RSA and ed25519 keys, covering both encrypted and unencrypted scenarios.
🧰 Tools
🪛 Gitleaks (8.21.2)
8-8: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
14-14: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
20-20: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
26-26: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
package.json (4)
14-21: Verify compatibility with updated dependencies.Several dependencies have major version updates that could introduce breaking changes, particularly
socks-proxy-agent(4.0.1 → 8.0.5) which has a new constructor signature.Run the following script to check usage of SocksProxyAgent in the codebase:
#!/bin/bash # Description: Check for usage of SocksProxyAgent to ensure compatibility with new constructor signature rg -A 5 "SocksProxyAgent" src/
5-12: Great dual package setup with type definitions.The package now properly supports both ESM and CommonJS formats with the correct exports configuration, and includes TypeScript type definitions.
24-43: Comprehensive test coverage with different formats.The updated scripts section provides extensive test coverage across different module formats (ESM/CJS) and test scenarios (static, live, WebSocket).
62-94: Updated dev dependencies with modern tooling.The development dependencies have been updated to include modern tools like TypeScript, ESLint, and Rollup, which will improve code quality and developer experience.
README.md (8)
92-98: CommonJS Example is Well-Illustrated
The “Getting started (CJS)” example (lines 92–98) clearly shows how to configure and instantiate the Binance API. The configuration options (APIKEY, APISECRET, test mode) are self-explanatory.
270-287: Futures Historical Bulk Data Download API Examples
The “Get Download ID” (lines 273–281) and “Get Download Link” (lines 283–287) examples clearly illustrate how to retrieve historical bulk data. The use of parameters like startTime, endTime, and dataType is well demonstrated.
402-493: Spot Trading and Account Information Examples
The examples in the “Binance API (Spot Trading)” section (lines 402–493) are clear. They demonstrate how to get latest prices, balances, and bid/ask details. The dual examples for getting data using callbacks versus async/await (if applicable) are helpful; just ensure that the sample responses and callback signatures remain in sync with the latest API behavior.🧰 Tools
🪛 LanguageTool
[uncategorized] ~483-~483: You might be missing the article “a” here.
Context: ....00035735</details> #### Getting list of current balancesjavascript binan...(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 markdownlint-cli2 (0.17.2)
404-404: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
1572-1610: Trade History and Open Orders Examples
The examples demonstrating how to retrieve trade history, open orders, and checking order status (lines 1572–1610) are functional and clear. They effectively showcase how to interact with order-related endpoints.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1605-1605: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
1728-1757: Additional WebSocket Examples for Candlesticks, Trades, and miniTicker
The blocks covering candlestick updates (lines 1728–1741), trade updates (lines 1744–1750), and miniTicker streams (lines 1752–1757) are concise and serve as very good examples of real-time data handling via WebSockets. These examples clarify the payload structure very well.
1899-1934: Deposit/Withdrawal Examples are Clear and Actionable
The examples for obtaining deposit addresses, deposit/withdraw history, and initiating withdrawals (with and without address tags) (lines 1899–1934) are well-documented. They provide users with clear guidance on using finance-related endpoints.
1949-1955: Universal Transfer Example Added
The “Universal Transfer / Internal Wallet Transfer” example (lines 1949–1955) is a neat addition that shows how to transfer funds between account types. The use of a clear ENUM ("MAIN_UMFUTURE") in the example is appreciated.
2177-2184: Contribution and Sponsor Section Engagement
The final “Contribution” section (lines 2177–2184) is engaging and visually appealing. It encourages community involvement with clear calls to action. Please verify the correctness of the external links and badges.🧰 Tools
🪛 LanguageTool
[uncategorized] ~2180-~2180: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...w) and give us a PR with your bugfix or improvement after. We love it ❤️ 
src/types.ts (1)
236-250: Duplicate “CancelOrder” interface definitions.
These two interfaces share the same name and identical properties, which can lead to inconsistent type references. Use a single definition or a unified approach to avoid confusion.- export interface CancelOrder { - ... - } ... - export interface CancelOrder { - ... - } + // Remove the second interface or merge them into a single definitionAlso applies to: 323-337
| status: string | ||
| timeInForce: string |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mismatched field types for “status” and “timeInForce.”
These fields accept string, but OrderStatus and TimeInForce types already exist to provide stronger type safety. Consider applying the appropriate types here.
- status: string
- timeInForce: string
+ status: OrderStatus
+ timeInForce: TimeInForceAlso applies to: 333-334
| futuresTicker(symbol?: string, callback?: Callback) | ||
| futuresMiniTicker(symbol?: string, callback?: Callback) | ||
| futuresAggTrades(symbols: string[] | string, callback: Callback) | ||
| futuresMarkPrice(symbol?: string, callback?: Callback, speed?: string) | ||
| futuresLiquidation(symbol?: string, callback?: Callback) | ||
| futuresTicker(symbol?: string, callback?: Callback) |
There was a problem hiding this comment.
Remove duplicate “futuresTicker” method signatures.
Lines 193 (futuresTicker(...)) and 198 (futuresTicker(...)) declare the same method name within the interface, which may cause confusion and TypeScript errors. Remove or rename one of them.
- futuresTicker(symbol?: string, callback?: Callback)
...
- futuresTicker(symbol?: string, callback?: Callback)
+ // Remove or rename one of these to avoid duplicationCommittable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit