-
Notifications
You must be signed in to change notification settings - Fork 255
hlp/sdk-finalize-and-quote-asset-update #1874
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
e7ea8ff to
7ecbe53
Compare
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
…if an asset is quote asset Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
31a2e84 to
f187628
Compare
🚨 E2E Tests FailedThe E2E tests failed during CI. These tests validate real blockchain interactions and may fail due to:
This is non-blocking and does not prevent merging. Check the action logs above for detailed failure information. |
tinom9
left a comment
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.
🔝
packages/hyperliquid-composer/src/commands/core-spot-deployment.ts
Outdated
Show resolved
Hide resolved
| } catch (error) { | ||
| logger.error(`Failed to check quote asset status: ${error}`) | ||
| process.exit(1) | ||
| } |
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.
I understand we want to be try-catching to avoid ugly errors from node.
Can we do it once at the CLI entrypoint?
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.
imo cli should at-most validate inputs and is a wrapper to commands
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.
I would prefer consistency on how we handle process terminations on errors for cases like this at the CLI level, as we're don't know what exactly went wrong.
Still a nit though, can be disregarded.
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
Signed-off-by: shankar <[email protected]>
🚨 E2E Tests FailedThe E2E tests failed during CI. These tests validate real blockchain interactions and may fail due to:
This is non-blocking and does not prevent merging. Check the action logs above for detailed failure information. |
Signed-off-by: shankar <[email protected]>
🚨 E2E Tests FailedThe E2E tests failed during CI. These tests validate real blockchain interactions and may fail due to:
This is non-blocking and does not prevent merging. Check the action logs above for detailed failure information. |
Signed-off-by: shankar <[email protected]>
🚨 E2E Tests FailedThe E2E tests failed during CI. These tests validate real blockchain interactions and may fail due to:
This is non-blocking and does not prevent merging. Check the action logs above for detailed failure information. |
ravinagill15
left a comment
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.
Looks good, minor nits, no need to update in this PR
| const COREWRITER_ADDRESS = '0x3333333333333333333333333333333333333333' | ||
| const FINALIZE_EVM_CONTRACT_ACTION_TYPE_PREFIX = '0x01000008' | ||
| const ACTION_TYPE_FINALIZE = 1 |
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.
nit: can move this to a constants.ts file
| const hypeTokenIndex = isTestnet ? HYPE_INDEX.TESTNET : HYPE_INDEX.MAINNET | ||
|
|
||
| // Get all HYPE trading pairs with metadata | ||
| const { pairs, tokens } = await getSpotPairsWithMetadata(isTestnet, hypeTokenIndex, logLevel) |
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.
potentially wrap with try/catch for in case of API failures
🚨 E2E Tests FailedThe E2E tests failed during CI. These tests validate real blockchain interactions and may fail due to:
This is non-blocking and does not prevent merging. Check the action logs above for detailed failure information. |
SDK Updates
Quote Assets
USDeis now a known quote asset on mainnet.Hyperliquid's quote asset protocol automatically deploys a
HYPE/ASSETspot market pair when an asset is promoted to a quote asset. Based on this we can assert that theASSETof everyHYPE/ASSETspot pair is a quote asset.New command added to check if a given core spot is a quote asset.
This logic is applied to
registerSpot(part of core spot deploy) and composer deployments to ensure that users deploy FeeToken or regular composers.EVM<>Core Linking
Renamed files from register* to link* to match hyperliquid's documentation.
Also created a new command to generate a
CoreWritercompatiblefinalizeEvmContractpayload that needs to be signed and submitted toCoreWriter(0x3333333333333333333333333333333333333333)refer: https://hyperliquid.gitbook.io/hyperliquid-docs/for-developers/hyperevm/interacting-with-hypercore#corewriter-contract
Docs update
For quote asset and new commands
Signed-off-by: shankar [email protected]