Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Submit transactions in bulk #219

Merged
merged 63 commits into from
Sep 2, 2023
Merged

Submit transactions in bulk #219

merged 63 commits into from
Sep 2, 2023

Conversation

ThibautBremand
Copy link
Collaborator

@ThibautBremand ThibautBremand commented Jul 30, 2023

  • Allow to submit a bulk of raw transaction payloads.
  • Add a Permissions view to enable or disable the permission to submit transactions in bulk
  • Use noWait (boolean) and onError (string) in the submitTransactionsBulk route to control if we want to wait for the tx hashes or not, and if we want to continue or abort when one tx fails
  • Resolve NFTs in the submitTransactionBulk view for NFT related transactions (mint, burn, NFT offers)
  • Docs: Add submitTransactionsBulk doc gemwallet-website#40

@ThibautBremand ThibautBremand added the enhancement New feature or request label Jul 30, 2023
@ThibautBremand ThibautBremand self-assigned this Jul 30, 2023
@ThibautBremand ThibautBremand force-pushed the feat/bulk-submit-tx branch 6 times, most recently from 6833341 to b0c736a Compare August 4, 2023 08:23
@ThibautBremand ThibautBremand force-pushed the feat/bulk-submit-tx branch 10 times, most recently from ea475e3 to 56a97c1 Compare August 13, 2023 08:42
@ThibautBremand ThibautBremand marked this pull request as ready for review August 13, 2023 08:42
@ThibautBremand ThibautBremand force-pushed the feat/bulk-submit-tx branch 7 times, most recently from d5d5da1 to 7a89736 Compare August 17, 2023 20:04
Copy link
Contributor

@florent-uzio florent-uzio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job I left a few comments but I can have another look later. Let me know.

packages/constants/src/network/network.constant.ts Outdated Show resolved Hide resolved
packages/constants/src/payload/payload.types.ts Outdated Show resolved Hide resolved
};

const results: TransactionBulkResponse[] = [];
for (const tx of payload.transactions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop will send the requests one by one which is slow.
Have you thought about using Promise.all or Promise.allSettled() to send them in parallel and make the whole process faster?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried with Promise.all but it was only really submitting the first transaction of each batch. My assumption was that I needed to throttle

});
};

export const loadFromChromeLocalStorage = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about sth like this? You can also implement your delete logic

/**
 * Retrieve an item from the local storage.
 *
 * @param {string} key
 * @returns Any value.
 */
export const getLocalStorageItem = <T = unknown>(
  key: string,
): T | undefined => {
  const value = localStorage.getItem(key)

  if (value === null || value === 'undefined') {
    return undefined
  }

  try {
    return JSON.parse(value) as T
  } catch (_err) {
    return value as unknown as T
  }
}

@ThibautBremand ThibautBremand force-pushed the feat/bulk-submit-tx branch 2 times, most recently from 3c78650 to c47d0b3 Compare August 20, 2023 12:04
@ThibautBremand ThibautBremand merged commit 8e3b64f into master Sep 2, 2023
3 checks passed
@ThibautBremand ThibautBremand deleted the feat/bulk-submit-tx branch September 2, 2023 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants