tiagosiebler / binance

Node.js & JavaScript SDK for Binance REST APIs & WebSockets, with TypeScript & browser support, integration tests, beautification & more.
MIT License
751 stars 266 forks source link

Feature: added a function that gets subAccount deposit history from s… #274

Closed MartoMcfly closed 1 year ago

MartoMcfly commented 1 year ago

…api/v1/broker/subAccount/depositHist

Signed-off-by: MartoMcfly martinpelaez@hotmail.es

Summary

Feature: added a function that gets subAccount deposit history from sapi/v1/broker/subAccount/depositHist

Additional Information

No breaking changes or dependencies added on this pull request

Xavier59 commented 1 year ago

Response type different than the one described in api documentation. Which one is correct ?

https://binance-docs.github.io/Brokerage-API/Brokerage_Operation_Endpoints/#get-sub-account-deposit-history

@MartoMcfly

Xavier59 commented 1 year ago

Can confirm typing were incorrect. Correct typings:

  getBrokerSubAccountDepositHistory(
    params?: GetBrokerSubAccountDepositHistoryParams
  ): Promise<BrokerSubAccountDepositHistory[]> {
    return this.getPrivate('sapi/v1/broker/subAccount/depositHist', params);
  }
export interface BrokerSubAccountDepositHistory {
    depositId: number,
    subAccountId: string,
    amount: string,
    coin: string,
    network: string,
    status: number,
    address: string,
    addressTag: string,
    txId: string,
    insertTime: number,
    sourceAddress: string,
    confirmTimes: string
}

Note: Binance documentation is a bit outdated too. Query returns an undocumented depositId attribute.

@tiagosiebler @MartoMcfly

tiagosiebler commented 1 year ago

Hey @Xavier59 almost missed this - silly question at a glance, does this mean that the typing in this SDK needs an update for that interface?

Xavier59 commented 1 year ago

Yes @tiagosiebler , this interface need an update with the model I gave above :)

tiagosiebler commented 1 year ago

Yes @tiagosiebler , this interface need an update with the model I gave above :)

Thanks @Xavier59 - fixed via #328 in v3.6.0. Should be on npm shortly. In future, if you comment somewhere like here and I potentially missed it, please feel free to open a new issue. I read your comments a while back but then totally forgot about it since it was hidden away in this thread. Thanks for sharing the corrected type too!