search

secretkeylabs / xverse-core

Core library for Xverse wallet
Other
38 stars 19 forks source link

Initial comments #2

Closed yknl closed 1 year ago

yknl commented 2 years ago
  1. Why are the API functions under currency?
  2. How come there aren't any Stacks API functions?
  3. STXNFT has been renamed to Gamma, so we should rename the API function accordingly
  4. transaction should be named transactions
  5. I see the index.ts file is empty, how are the functions exported?
  6. I would group the API calls by the provider. So for example, all blockcypher API calls would be in the same file.
  7. The types should probably be in the same file as the API calls, to avoid multiple imports.
Sam-Lam-Secret-Key-Labs commented 2 years ago
  1. Those are currency related APIs e.g. getting BTC rates with Fiat etc.
  2. There are, e.g. under the wallet/api folder there is stacks.ts which contains stacks api Calls.
  3. OK
  4. OK
  5. Im still deciding the way to export, maybe we can do it like stack.js and have all module exported with different paths.
  6. My idea was to have different providers under functionality because it might not be very clear which provider provides with APIs.
  7. Some types are shared between 2 files or even by the library users so there will be import anyway, I made this decision to keep all types away so library users can also import the types directly.
yknl commented 2 years ago

Additional recommendations:

I would logically separate the library functions into the following categories, similar to how it is currently done in ./common of the Xverse mobile app repo:

  1. API calls ./api - Network requests to that fetch data
  2. Transactions ./transactions - Functions that deal with generation or parsing of transactions
  3. Wallet ./wallet - Functions that deal with key derivation, encryption etc.
  4. Utils ./utils - Util functions and everything else

Further, within ./api, the actual API network call functions should be separated by source. For example, all of calls to xverse-api can be under ./api/xverse, similarly ./api/gamma, ./api/blockcypher, ./api/stacks-node and etc. The API exposed to the apps (xverse mobile and desktop) should not be tied to a particular source. Rather they should be named by function.

For example: A function getNFTDetails() will use an API network call function from ./api/gamma. If the API source was ever to change from Gamma to something else, we can still make sure that getNFTDetails()'s return type is the same, avoiding breaking changes on the app side.

Within ./transactions, we can categorize by functionality into btc, stacks and later lightning.