-
Notifications
You must be signed in to change notification settings - Fork 19
Allow creating an allocation with user-specified parameters #1082
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Oriol Muñoz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request implements the "Allocations create" feature by introducing backend support for amulet allocation operations and the corresponding frontend components and API endpoints. Key changes include:
- New backend methods and data types (e.g. EnqueuedAmuletAllocationOperation, AmuletAllocationOperationBatch) added to TreasuryService.scala to handle allocation operations.
- Updates to the API specification (wallet-internal.yaml) to support the new allocation endpoints.
- Frontend modifications including new routes, components (CreateAllocation, AmountInput) and context updates to integrate allocation creation into the wallet UI.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/treasury/TreasuryService.scala | Adds methods to enqueue and execute amulet allocation operations. |
apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/admin/http/HttpWalletHandler.scala | Implements new HTTP handler endpoints for allocation operations. |
apps/wallet/src/main/openapi/wallet-internal.yaml | Extends OpenAPI schema with allocateAmulet endpoints and response definitions. |
apps/wallet/frontend/src/routes/allocations.tsx | Creates a new frontend route for allocations. |
apps/wallet/frontend/src/contexts/WalletServiceContext.tsx | Adds context support for creating allocations. |
apps/wallet/frontend/src/components/CreateAllocation.tsx | Introduces a new React component to create allocations. |
apps/wallet/frontend/src/components/AmountInput.tsx | Updates the amount input component to be reusable across views. |
apps/wallet/frontend/src/App.tsx | Integrates the new Allocations route into the main app. |
apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SpliceLedgerConnection.scala | Updates the CommandId signature and logic to support improved dedup handling. |
Comments suppressed due to low confidence (2)
apps/wallet/src/main/openapi/wallet-internal.yaml:1432
- The dummy field in AllocationInstructionResultFailed is used as a placeholder for an empty object. Consider adding a more descriptive comment or an alternative approach so that future developers understand its purpose and any plans for improvement.
dummy: # cannot define an empty object for some reason
apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SpliceLedgerConnection.scala:1310
- [nitpick] The updated handling of the 'discriminator' in CommandId now incorporates each string's length before appending. Please ensure that any consumers relying on the previous string format are updated and that the new format is documented clearly.
.appendedAll(discriminator.flatMap(str => Seq(str.length.toString, str)))
Signed-off-by: Oriol Muñoz <[email protected]>
...a/org/lfdecentralizedtrust/splice/integration/tests/AllocationsFrontendIntegrationTest.scala
Outdated
Show resolved
Hide resolved
apps/app/src/test/scala/org/lfdecentralizedtrust/splice/util/WalletFrontendTestUtil.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, thank you!
As discussed:
- Please add an integration test for the backend that does not go through the frontend
- In the future keep the backend changes and frontend changes in separate PRs
...mmon/src/main/scala/org/lfdecentralizedtrust/splice/environment/SpliceLedgerConnection.scala
Outdated
Show resolved
Hide resolved
...a/org/lfdecentralizedtrust/splice/integration/tests/AllocationsFrontendIntegrationTest.scala
Show resolved
Hide resolved
Signed-off-by: Oriol Muñoz <[email protected]>
Signed-off-by: Oriol Muñoz <[email protected]>
Signed-off-by: Oriol Muñoz <[email protected]>
Signed-off-by: Oriol Muñoz <[email protected]>
Signed-off-by: Oriol Muñoz <[email protected]>
Signed-off-by: Oriol Muñoz <[email protected]>
Signed-off-by: Oriol Muñoz <[email protected]>
fb089e2
to
c0c1e51
Compare
Signed-off-by: Oriol Muñoz <[email protected]>
Fixes #451