feat(multichain-account-service): add :walletStatusChange event + mutex for concurrent mutable operations
#6527
Conversation
| if (this.#isAlignmentInProgress) { | ||
| return; // Prevent concurrent alignments | ||
| } | ||
|
|
There was a problem hiding this comment.
Actually, removed the old logic mainly cause this could be misleading.
If an alignement was in progress, calling alignGroups would have returned right away, so you needed to check getIsAlignmentInProgress to know if something was still running.
With the new #withLock mechanisms. All those operations will naturally be "queued" until the lock got released. So, we won't be running those concurrently, but more sequentially instead.
packages/multichain-account-service/src/MultichainAccountWallet.ts
Outdated
Show resolved
Hide resolved
| * wallet. Account groups will either be created or deleted during | ||
| * this operation. | ||
| */ | ||
| OperationInProgress = 'operation-in-progress', |
There was a problem hiding this comment.
I struggle to find a good name for this one... Maybe we should also split create/delete into 2 different status 🤔
(deleting is not possible for now though)
There was a problem hiding this comment.
"CrudOperation"? "Mutation" ? No clue either 😅
There was a problem hiding this comment.
I actually went with in-progress:create-accounts, mainly because we don't have "delete operations".
packages/multichain-account-service/src/MultichainAccountWallet.ts
Outdated
Show resolved
Hide resolved
| ); | ||
| return await operation(); | ||
| } finally { | ||
| this.#status = MultichainAccountWalletStatus.Ready; |
There was a problem hiding this comment.
Don't we need a Error state?
There was a problem hiding this comment.
Good question 🤔 the main problem I can see with this is... How to get back into the Ready state if that happens? 😅
There was a problem hiding this comment.
After thinking about it, I don't think we need any error state for the moment.
ccharly
force-pushed
the
feat/multichain-account-service-operation-events
branch
from
db27b9a to
207f2ea
Compare
ccharly
force-pushed
the
feat/multichain-account-service-operation-events
branch
from
e16f62a to
af6b40e
Compare
ccharly
changed the base branch from
main
to
chore/bump-multichain-accounts-deps
ccharly
force-pushed
the
feat/multichain-account-service-operation-events
branch
from
af6b40e to
cc8a5bd
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
ccharly
force-pushed
the
feat/multichain-account-service-operation-events
branch
from
0bab851 to
a30e2a8
Compare
…utex for concurrent mutable operations
ccharly
force-pushed
the
feat/multichain-account-service-operation-events
branch
from
a30e2a8 to
eb9db81
Compare
| ) { | ||
| const release = await this.#lock.acquire(); | ||
| try { | ||
| this.#status = status; |
There was a problem hiding this comment.
Don't we need to validate it's a valid status string, at runtime?
ccharly
deleted the
feat/multichain-account-service-operation-events
branch
Explanation
Wallet operations are not behind a mutex. This ensure that only one mutable operation can be executed at a time.
Also publish new event, so we know in which state a wallet is.
Note
A follow-up PR will be using those events to add a new status in the
account-tree-controller.Depends on this PR to be merged first:
References
N/A
Checklist