Skip to content

Refactor: Use new Wallet instances in parallel calls (COIN-5426) #6978

@bhavidhingra

Description

@bhavidhingra

Feature Description

Refactor the bulkSignAccountBasedMessagesWithProvider function in modules/sdk-core/src/bitgo/walletUtil/signAccountBasedMidnightClaimMessages.ts to ensure that new instances of Wallet are used for each parallel call.

  • Extract constructor params from the original wallet instance.
  • For each message to be processed in parallel, instantiate a new Wallet object with the extracted params.
  • Perform signing/building using the new wallet instance per message.

Also, write comprehensive tests to verify the new behavior and cover edge cases.

Motivation

  • Prevent shared state issues or side effects when processing messages in parallel.
  • Align with COIN-5426 requirements.

File changed:

  • modules/sdk-core/src/bitgo/walletUtil/signAccountBasedMidnightClaimMessages.ts

Relevant diff:

diff --git a/modules/sdk-core/src/bitgo/walletUtil/signAccountBasedMidnightClaimMessages.ts b/modules/sdk-core/src/bitgo/walletUtil/signAccountBasedMidnightClaimMessages.ts
index 47f39c179..74584b055 100644
--- a/modules/sdk-core/src/bitgo/walletUtil/signAccountBasedMidnightClaimMessages.ts
+++ b/modules/sdk-core/src/bitgo/walletUtil/signAccountBasedMidnightClaimMessages.ts
@@ -1,4 +1,4 @@
-import { IWallet } from '../wallet';
+import { Wallet, IWallet } from '../wallet';
 import { MessageStandardType } from '../utils';
 import { MidnightMessageProvider } from './midnightMessageProvider';
 import { IMessageProvider, MessageInfo } from './iMessageProvider';
@@ -27,17 +27,25 @@ async function bulkSignAccountBasedMessagesWithProvider(
   const failedAddresses: string[] = [];
   const txRequests: Record<string, unknown>[] = [];
 
+  // Gather all messages to process (flatten all batches)
+  let allMessages: MessageInfo[] = [];
   let messages: MessageInfo[] = await provider.getMessagesAndAddressesToSign();
   while (messages.length > 0) {
-    // Sign/build all messages in parallel
-    const results = await Promise.all(
-      messages.map((messageInfo) => signOrBuildMessage(wallet, messageInfo, messageStandardType, walletPassphrase))
-    );
-    // Process results and update counters
-    processResults(results, txRequests, failedAddresses);
-    // Get next batch of messages
+    allMessages = allMessages.concat(messages);
     messages = await provider.getMessagesAndAddressesToSign();
   }
+
+  // Extract wallet constructor params
+  const { bitgo, baseCoin, _wallet: walletData } = wallet as Wallet;
+
+  // Process all messages in parallel, each with a new Wallet instance
+  const results = await Promise.all(
+    allMessages.map(async (messageInfo) => {
+      const newWallet = new Wallet(bitgo, baseCoin, walletData);
+      return signOrBuildMessage(newWallet, messageInfo, messageStandardType, walletPassphrase);
+    })
+  );
+  processResults(results, txRequests, failedAddresses);
   return { failedAddresses, txRequests };
 }
 

Action Items

  • Refactor to use new Wallet instances for each parallel operation as per above.
  • Write/modify tests to validate the new behavior and prevent regression.
  • Ensure robust error handling and maintain performance.

Metadata

Metadata

Assignees

No fields configured for Feature.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions