feat: return groupId when adding groups (CPL-145)#202
Conversation
Modify the Solidity addGroup function to return the on-chain group ID (uint256). Update ABI, generated Rust bindings, and make send_transaction generic over Detokenize. The Rust add_group function simulates the call first to capture the group ID, then sends the real transaction. Adds AddGroupResponse with success + group_id fields. Includes a fix for signer pool leak if the simulation call fails. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update core_sdk.js addGroup to document the new AddGroupResponse type. Dashboard now shows the created group ID in the success toast. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add AddGroupResponse type to K6 client. Integration and security tests now extract group_id directly from the addGroup response instead of calling listGroups, simplifying setup and removing race conditions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show the new {"success":true,"group_id":"1"} response in API docs.
Remove the now-unnecessary listGroups step from quickstart examples.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CPL-145 Return groupId when adding groups
Update contracts "add Group" call, to return the group id. Persist this change through the stack all the way to the api & dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR updates the AccountConfig “add group” flow end-to-end (Solidity → ABI/bindings → API server → SDK/UI/tests/docs) so that creating a group returns the newly created on-chain group_id, removing the need for a subsequent list_groups lookup.
Changes:
- Solidity
addGroupnow returnsuint256(the new group ID), with ABI JSON + generated Rust bindings updated accordingly. - API server
add_groupnow returnsAddGroupResponse { success, group_id }, with Rust implementation simulating the call to capture the ID before sending the real transaction (and fixing signer release on simulation error). - Frontend SDK/UI, k6 tests, and docs updated to consume/display the returned
group_id.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lit-static/dapps/dashboard/app.js | Uses returned group_id to show a success toast including the created group ID. |
| lit-static/core_sdk.js | Updates addGroup() JSDoc return type to AddGroupResponse. |
| lit-api-server/src/core/v1/models/response.rs | Introduces AddGroupResponse model; adjusts AccountOpResponse comment. |
| lit-api-server/src/core/v1/endpoints/account_management.rs | Changes /add_group endpoint response type to AddGroupResponse. |
| lit-api-server/src/core/account_management.rs | Returns group_id from accounts::add_group() via AddGroupResponse. |
| lit-api-server/src/accounts/signable_contract.rs | Makes send_transaction generic over Detokenize to support non-() return types. |
| lit-api-server/src/accounts/mod.rs | Implements simulate-then-send add_group() returning U256 group ID; releases signer on sim error. |
| lit-api-server/src/accounts/contracts/account_config_contract.rs | Updates generated binding for add_group return type to U256. |
| lit-api-server/src/accounts/contracts/AccountConfig.json | Updates ABI for addGroup to include uint256 return output. |
| lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/WritesFacet.sol | addGroup now returns (uint256) and returns account.groupCount. |
| k6/litApiServer.ts | Updates OpenAPI-generated types so /add_group returns AddGroupResponse. |
| k6/correctness/integration.spec.ts | Uses returned group_id directly; keeps listGroups only for verification. |
| k6/correctness/api-key-security.spec.ts | Uses returned group_id for setup/cleanup instead of listGroups ID discovery. |
| docs/management/api_direct.mdx | Updates examples to use group_id from add_group response. |
| docs/api_direct.mdx | Updates examples to use group_id from add_group response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * POST /core/v1/add_group | ||
| * Add a group to an account with permitted action hashes and PKP hashes. | ||
| * @param {AddGroupOptions} options | ||
| * @returns {Promise<AccountOpResponse>} | ||
| * @returns {Promise<AddGroupResponse>} Response with `success` and `group_id`. | ||
| */ |
There was a problem hiding this comment.
addGroup() now documents returning AddGroupResponse, but this type isn’t defined anywhere in the SDK JSDoc, so consumers relying on JSDoc/TS inference will see an unknown type. Add a @typedef {Object} AddGroupResponse (with success and group_id) alongside the other response typedefs, and consider updating the existing AccountOpResponse typedef description to no longer mention add_group since it no longer matches this endpoint’s response shape.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Done in commit 0facc5c:
- Added
@typedef {Object} AddGroupResponse(withsuccessboolean andgroup_idstring) alongside the other response typedefs incore_sdk.js. - Updated
AccountOpResponsetypedef to remove the staleadd_groupmention — it now reads "Response for operations that return onlysuccess" with the full list of applicable endpoints. - Updated the
AccountOpResponsedoc comment inresponse.rsto match the same generic description.
Note: the commit is ready locally but the branch protection rule is blocking direct pushes — a repo admin will need to allow the push or relax the rule for this branch.
AccountOpResponsedoc commentAccountOpResponsedoc comment inresponse.rsto be generic (covers allsuccess-only endpoints)AddGroupResponsetypedef tocore_sdk.js(withsuccessandgroup_idproperties)AccountOpResponsetypedef incore_sdk.jsto removeadd_groupmention⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.