Skip to content

Comments

fix: prevent nc history race condition when running more than one request at once#830

Merged
pedroferreira1 merged 5 commits intomasterfrom
fix/nc-history-race-condition
Jan 28, 2026
Merged

fix: prevent nc history race condition when running more than one request at once#830
pedroferreira1 merged 5 commits intomasterfrom
fix/nc-history-race-condition

Conversation

@pedroferreira1
Copy link
Member

Motivation

The nano contract history list shows duplicate transactions. One possibility is a race condition when loading the before history more than once, and we don't deduplicate the elements.

Deduplicating it is more expensive than having a lock to prevent concurrent calls. Below a better analysis of the scenario from claude.

Acceptance Criteria

  • Create lock to prevent concurrent requests.

Example of race condition

The Bug: Race Condition in Saga Guard                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                             
  Looking at src/sagas/nanoContract.js:333-343:                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                             
  export function* requestHistoryNanoContract({ payload }) {                                                                                                                                                                                                                 
    const { ncId, before, after } = payload;                                                                                                                                                                                                                                 
                                                                                                                                                                                                                                                                             
    const historyMeta = yield select((state) => state.nanoContract.historyMeta);                                                                                                                                                                                             
    if (historyMeta[ncId] && historyMeta[ncId].isLoading) {                                                                                                                                                                                                                  
      // Do nothing if nano contract already loading...                                                                                                                                                                                                                      
      return;                                                                                                                                                                                                                                                                
    }                                                                                                                                                                                                                                                                        
    yield put(nanoContractHistoryLoading({ ncId }));  // <-- Race window here!                                                                                                                                                                                               
    // ...                                                                                                                                                                                                                                                                   
  }                                                                                                                                                                                                                                                                          
                                                                                                                                                                                                                                                                             
  There's a race window between yield select() and yield put(). In Redux-Saga, each yield is a suspension point where other sagas can run.                                                                                                                                   
                                                                                                                                                                                                                                                                             
  How Duplicates Occur                                                                                                                                                                                                                                                       
                                                                                                                                                                                                                                                                             
  1. Component mounts with txHistory = [A, B, C]                                                                                                                                                                                                                             
  2. useEffect dispatches nanoContractHistoryRequest({ ncId, before: A.txId })                                                                                                                                                                                               
  3. Saga1 starts, yield select() → isLoading: false                                                                                                                                                                                                                         
  4. User immediately pulls to refresh (or React StrictMode double-mounts)                                                                                                                                                                                                   
  5. handleNewerTransactions dispatches nanoContractHistoryRequest({ ncId, before: A.txId }) — same parameters!                                                                                                                                                              
  6. Saga2 starts, yield select() → isLoading: false (Saga1 hasn't put yet!)                                                                                                                                                                                                 
  7. Both sagas pass the guard and proceed                                                                                                                                                                                                                                   
  8. Both fetch with before: A.txId, both get [D, E]                                                                                                                                                                                                                         
  9. Saga1 dispatches nanoContractHistorySuccess({ ncId, beforeHistory: [D, E] })                                                                                                                                                                                            
  10. Reducer: [D, E] + [A, B, C] = [D, E, A, B, C]                                                                                                                                                                                                                          
  11. Saga2 dispatches nanoContractHistorySuccess({ ncId, beforeHistory: [D, E] })                                                                                                                                                                                           
  12. Reducer: [D, E] + [D, E, A, B, C] = [D, E, D, E, A, B, C] ← DUPLICATES!

@pedroferreira1 pedroferreira1 self-assigned this Jan 27, 2026
@pedroferreira1 pedroferreira1 moved this from Todo to In Progress (Done) in Hathor Network Jan 27, 2026
Comment on lines -181 to +184
ncId: lastTx.ncId,
ncId,
Copy link
Member Author

Choose a reason for hiding this comment

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

The history of a nano returns also txs of other contracts that execute methods of this contract, so lastTx.ncId was a different of ncId in some of the txs.

@pedroferreira1 pedroferreira1 moved this from In Progress (Done) to In Review (WIP) in Hathor Network Jan 28, 2026
log.debug('Halting processing for nano contract transaction history request while it is loading...');
return;
// Determine request type: 'before' (newer txs), 'after' (older txs), or 'initial' (full reload)
let requestType = 'initial';
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion(non-blocking): Create a constant or enum for this hardcoded string

Copy link
Member Author

Choose a reason for hiding this comment

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

Much better, thanks! c99b622

* @param {string} ncId Nano Contract ID
* @returns {Set<string>}
*/
function getLoadingSet(ncId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion(non-blocking): Rename to getLoadingLockSet and the variables that fetch from here to loadingLock.

It makes it easier to understand, since most other places with loading in this codebase refer to a component state related to a spinner for user feedback.

Copy link
Member Author

Choose a reason for hiding this comment

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

Much better, thanks! c99b622

@tuliomir tuliomir moved this from In Review (WIP) to In Review (Done) in Hathor Network Jan 28, 2026
@github-project-automation github-project-automation bot moved this from In Review (Done) to In Review (WIP) in Hathor Network Jan 28, 2026
@tuliomir tuliomir moved this from In Review (WIP) to In Review (Done) in Hathor Network Jan 28, 2026
@pedroferreira1 pedroferreira1 merged commit 6324c62 into master Jan 28, 2026
1 check passed
@pedroferreira1 pedroferreira1 deleted the fix/nc-history-race-condition branch January 28, 2026 22:12
@github-project-automation github-project-automation bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Waiting to be deployed

Development

Successfully merging this pull request may close these issues.

3 participants