Skip to content

feat(tangle-cloud): blueprint deployment request args configuration & finalization #2957

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

Merged

Conversation

danielbui12
Copy link
Contributor

@danielbui12 danielbui12 commented Apr 7, 2025

Summary of changes

Provide a detailed description of proposed changes.

  • Implement request Args configuration step
  • Implement finalization step

Proposed area of change

Put an x in the boxes that apply.

  • apps/tangle-dapp
  • apps/tangle-cloud
  • apps/leaderboard
  • libs/tangle-shared-ui
  • libs/ui-components

Associated issue(s)

Specify any issues that can be closed from these changes (e.g. Closes #233).

Screen Recording

If possible provide screenshots and/or a screen recording of proposed change.

Screen.Recording.2025-04-12.at.09.39.12.mov

Copy link

netlify bot commented Apr 7, 2025

Deploy Preview for tangle-leaderboard ready!

Name Link
🔨 Latest commit 67d42c8
🔍 Latest deploy log https://app.netlify.com/sites/tangle-leaderboard/deploys/67fee9017f4e020008e5f0e4
😎 Deploy Preview https://deploy-preview-2957--tangle-leaderboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 7, 2025

Deploy Preview for tangle-dapp ready!

Name Link
🔨 Latest commit 67d42c8
🔍 Latest deploy log https://app.netlify.com/sites/tangle-dapp/deploys/67fee9018fda32000983a0ad
😎 Deploy Preview https://deploy-preview-2957--tangle-dapp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 7, 2025

Deploy Preview for tangle-cloud ready!

Name Link
🔨 Latest commit 67d42c8
🔍 Latest deploy log https://app.netlify.com/sites/tangle-cloud/deploys/67fee9019fb92900089c9676
😎 Deploy Preview https://deploy-preview-2957--tangle-cloud.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines 68 to 70
context.securityRequirements[index].minExposurePercent,
maxExposurePercent:
context.securityRequirements[index].maxExposurePercent,
Copy link
Member

Choose a reason for hiding this comment

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

Should have a check or assertion to ensure context.securityRequirements has the same length as context.assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to zod schema apps/tangle-cloud/src/utils/validations/deployBlueprint.ts:191:199

Context
> = useCallback(
async (context) => {
const api = await getApiPromise(wsRpcEndpoint);
Copy link
Member

Choose a reason for hiding this comment

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

To keep a single source of truth, we could pass the API to the factory instead of using getApiPromise (which caches it).

Another option is to investigate how TypeScript encodes these types, but that could take more time.

Copy link
Contributor Author

@danielbui12 danielbui12 Apr 13, 2025

Choose a reason for hiding this comment

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

Another option is to investigate how TypeScript encodes these types, but that could take more time

Dont think so 😂, cuz we generated types for easier usage, why dont we leverage it instead of try to re-write existing logic in PAPI

Copy link
Member

@AtelyPham AtelyPham left a comment

Choose a reason for hiding this comment

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

Left some change suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

The logic in useRestakeAssets is highly similar to existing logic. Refactor into a reusable hook and use it within useRestakeAssets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hook is used to get all asets not only restake assets

Comment on lines +15 to +24
const useServicesRegisterTx = () => {
const substrateTxFactory: SubstrateTxFactory<Context> = useCallback(
async (api, _activeSubstrateAddress, context) => {
const { blueprintIds, preferences, registrationArgs, amounts } = context;

// TODO: Find a better way to get the chain decimals
const decimals =
api.registry.chainDecimals.length > 0
? api.registry.chainDecimals[0]
: TANGLE_TOKEN_DECIMALS;
Copy link
Member

Choose a reason for hiding this comment

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

You can import the network and get the decimals from it: useNetworkStore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but it results in a recursive hook within a hook. @AtelyPham do you have any idea?

Copy link
Member

Choose a reason for hiding this comment

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

@danielbui12 using custom hooks within custom hooks is totally normal & common practice, if that's what you meant. Think about it like functions within functions -- it makes sense, it's simply building blocks reusing each other

Copy link
Member

Choose a reason for hiding this comment

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

any update here? @danielbui12

@danielbui12 danielbui12 requested a review from yuri-xyz April 15, 2025 04:20
@yuri-xyz
Copy link
Member

conflicts @danielbui12

@danielbui12
Copy link
Contributor Author

@yuri-xyz resolved. please take a time to verify my pr 🙏

@yuri-xyz
Copy link
Member

merging, if anything's left over, feel free to solve in your next pr

@yuri-xyz yuri-xyz merged commit 587c073 into develop Apr 15, 2025
20 checks passed
@yuri-xyz yuri-xyz deleted the daniel/blueprint-deployment-request-args-configuration branch April 15, 2025 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK] Instance Custom Parameters Inputs
4 participants