fix(identities): assign real nonce to identity txs instead of hardcoded 1#98
Conversation
…ed 1 inferIdentity (add/link) and removeIdentity (remove) both hardcoded nonce:1. That only validates for a fresh account (nonce 0): the first identity op succeeds and bumps the on-chain nonce, so the second (e.g. removing an XM wallet right after adding it) is rejected with '[NONCE ERROR] Nonce not assigned'. Both now fetch demos.getAddressNonce(address) and use nonce+1, matching every other native tx in the SDK (DemosTransactions.pay, nativeBridge, ContractDeployer).
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
WalkthroughIn ChangesDynamic nonce for identity transactions
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/abstraction/Identities.ts`:
- Line 142: The add/remove flow in Identities.ts is fetching the next nonce too
early because demos.confirm() only validates the tx and does not wait for
on-chain inclusion. Update the logic around the add transaction and the
subsequent getAddressNonce() call to wait until the add tx is actually landed,
using broadcastAndWait() or polling getTransactionStatus() first, so the remove
tx does not reuse the old nonce.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea42c7a3-2738-42a2-857e-7ed8ff0ac691
📒 Files selected for processing (1)
src/abstraction/Identities.ts
| // Use the address' current nonce (+1) like every other native tx in | ||
| // this SDK — hardcoding nonce:1 only validated for a fresh account and | ||
| // broke the second identity op (e.g. remove after add). | ||
| const nonce = await demos.getAddressNonce(address) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect confirm() and getAddressNonce() to confirm sequencing guarantees.
fd -t f 'demosclass.ts' --exec ast-grep outline {} \;
rg -nP --type=ts -C8 '\b(confirm|getAddressNonce)\s*\(' src/websdk/demosclass.tsRepository: kynesyslabs/sdks
Length of output: 3584
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the confirm path and nonce lookup behavior around the relevant methods.
ast-grep outline src/websdk/demosclass.ts --view expanded | sed -n '1,220p'
printf '\n--- confirm/getAddressNonce context ---\n'
sed -n '430,520p' src/websdk/demosclass.ts
printf '\n--- getAddressNonce context ---\n'
sed -n '1298,1335p' src/websdk/demosclass.ts
printf '\n--- DemosTransactions references ---\n'
rg -n -C 6 'class DemosTransactions|static confirm|confirm\(transaction|broadcastAndWait|broadcast\(' src/websdk/demosclass.tsRepository: kynesyslabs/sdks
Length of output: 10224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the confirm path and nonce lookup behavior around the relevant methods.
ast-grep outline src/websdk/demosclass.ts --view expanded | sed -n '1,220p'
printf '\n--- confirm/getAddressNonce context ---\n'
sed -n '430,520p' src/websdk/demosclass.ts
printf '\n--- getAddressNonce context ---\n'
sed -n '1298,1335p' src/websdk/demosclass.ts
printf '\n--- DemosTransactions references ---\n'
rg -n -C 6 'class DemosTransactions|static confirm|confirm\(transaction|broadcastAndWait|broadcast\(' src/websdk/demosclass.tsRepository: kynesyslabs/sdks
Length of output: 10224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the DemosTransactions implementation and inspect confirm/broadcast semantics.
fd -t f . src/websdk | sed -n '1,200p'
printf '\n--- search ---\n'
rg -n -C 8 'class DemosTransactions|confirm\(transaction|broadcastAndWait|getTransactionStatus|getAddressNonce' src/websdkRepository: kynesyslabs/sdks
Length of output: 25480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the DemosTransactions implementation and inspect confirm/broadcast semantics.
fd -t f . src/websdk | sed -n '1,200p'
printf '\n--- search ---\n'
rg -n -C 8 'class DemosTransactions|confirm\(transaction|broadcastAndWait|getTransactionStatus|getAddressNonce' src/websdkRepository: kynesyslabs/sdks
Length of output: 25480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the reviewed file and the transaction confirmation implementation.
ast-grep outline src/abstraction/Identities.ts --view expanded | sed -n '1,220p'
printf '\n--- identities excerpt ---\n'
sed -n '120,210p' src/abstraction/Identities.ts
printf '\n--- DemosTransactions.confirm implementation ---\n'
sed -n '1,360p' src/websdk/DemosTransactions.ts | sed -n '300,360p'Repository: kynesyslabs/sdks
Length of output: 8195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the reviewed file and the transaction confirmation implementation.
ast-grep outline src/abstraction/Identities.ts --view expanded | sed -n '1,220p'
printf '\n--- identities excerpt ---\n'
sed -n '120,210p' src/abstraction/Identities.ts
printf '\n--- DemosTransactions.confirm implementation ---\n'
sed -n '1,360p' src/websdk/DemosTransactions.ts | sed -n '300,360p'Repository: kynesyslabs/sdks
Length of output: 8195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the confirm implementation and nearby transaction flow.
rg -n -C 12 'confirm: async function|confirm\s*\(' src/websdk/DemosTransactions.ts
sed -n '240,340p' src/websdk/DemosTransactions.tsRepository: kynesyslabs/sdks
Length of output: 4390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the confirm implementation and nearby transaction flow.
rg -n -C 12 'confirm: async function|confirm\s*\(' src/websdk/DemosTransactions.ts
sed -n '240,340p' src/websdk/DemosTransactions.tsRepository: kynesyslabs/sdks
Length of output: 4390
Wait for the add tx to land before fetching the next nonce. demos.confirm() only runs confirmTx validation and returns immediately; it does not wait for on-chain inclusion. In the remove-after-add flow, use broadcastAndWait() or poll getTransactionStatus before calling getAddressNonce(), or the second tx can reuse the old nonce and collide.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/abstraction/Identities.ts` at line 142, The add/remove flow in
Identities.ts is fetching the next nonce too early because demos.confirm() only
validates the tx and does not wait for on-chain inclusion. Update the logic
around the add transaction and the subsequent getAddressNonce() call to wait
until the add tx is actually landed, using broadcastAndWait() or polling
getTransactionStatus() first, so the remove tx does not reuse the old nonce.
Greptile SummaryThis PR fixes nonce assignment for identity transactions. The main changes are:
Confidence Score: 5/5Safe to merge based on the scoped identity nonce change. The change is small and aligns identity transaction nonce handling with the existing native transaction pattern in the SDK.
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "fix(identities): assign real nonce to id..." | Re-trigger Greptile |
HaykK-Solicy
changed the title
|
2 participants
…ed 1
inferIdentity (add/link) and removeIdentity (remove) both hardcoded nonce:1. That only validates for a fresh account (nonce 0): the first identity op succeeds and bumps the on-chain nonce, so the second (e.g. removing an XM wallet right after adding it) is rejected with '[NONCE ERROR] Nonce not assigned'. Both now fetch demos.getAddressNonce(address) and use nonce+1, matching every other native tx in the SDK (DemosTransactions.pay, nativeBridge, ContractDeployer).
Summary by CodeRabbit