-
Notifications
You must be signed in to change notification settings - Fork 23
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
Release/main/20240611 #3494
Release/main/20240611 #3494
Conversation
[FeatFix][611] - fix switch chain
WalkthroughThe updates introduce new constants, pipes, and services, enhancing the handling of Ethereum-based tokens, particularly burnt tokens. Key changes include new error handling, asynchronous operations, and improved conditional rendering in components. These modifications aim to refine the user experience and ensure robust handling of Ethereum interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletService
participant TokenService
participant Component
User->>Component: Interacts with EVM component
Component->>WalletService: Call connectEvmWallet()
WalletService->>WalletService: Check chain connection
WalletService-->>Component: Return connection status
Component->>TokenService: Request token data
TokenService->>TokenService: Filter using TOKEN_EVM_BURNT
TokenService-->>Component: Return filtered token data
Component-->>User: Display token data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 8
Outside diff range and nitpick comments (7)
src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-read/evm-read.component.ts (3)
8-8
: Ensure proper encapsulation of service imports.Consider using absolute paths for consistency and to avoid potential issues with deeper directory structures.
Line range hint
56-56
: Optimize the use of spread syntax in accumulators.Using spread syntax in
.reduce
can lead to performance issues due to itsO(n^2)
complexity. Consider using.push
or direct assignment instead.- const group = abi.inputs.reduce((prevValue, curValue, idx) => { - const control = this.fb.control('', Validators.required); - return { - ...prevValue, - [curValue.name || idx]: control, - }; - }, {}); + const group = {}; + abi.inputs.forEach((curValue, idx) => { + group[curValue.name || idx] = this.fb.control('', Validators.required); + });
Line range hint
117-168
: RefactorhandleQueryContract
to improve error handling and connection logic.The method now includes better error handling and connection checks, which are crucial for robustness. However, consider abstracting the connection and error handling logic to reduce complexity and enhance readability.
async ensureConnected() { const connected = await this.walletService.connectToChain(); if (!connected) { throw new Error(`Please switch to ${this.environmentService.evmChainInfo.chain} chain.`); } } async handleQueryContract(jsonFragment: JsonFragmentExtends) { if (!jsonFragment || formGroup.invalid) return; jsonFragment.isValidate = true; const params = this.prepareParams(jsonFragment.inputs, formGroup.controls); try { await this.ensureConnected(); const contract = this.createContract(); if (!contract) return; jsonFragment.isLoading = true; const result = await contract[jsonFragment.name](...params); jsonFragment.result = result; } catch (error) { jsonFragment.error = { code: error.code, message: error.message }; } finally { jsonFragment.isLoading = false; } }src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-write/evm-write.component.ts (1)
Line range hint
74-74
: Optimize the use of spread syntax in accumulators.Using spread syntax in
.reduce
can lead to performance issues due to itsO(n^2)
complexity. Consider using.push
or direct assignment instead.- const group = inputs.reduce((prevValue, curValue) => { - const control = this.fb.control('', curValue.name === 'fund' ? null : Validators.required); - return { - ...prevValue, - [curValue.name]: control, - }; - }, {}); + const group = {}; + inputs.forEach((curValue) => { + group[curValue.name] = this.fb.control('', curValue.name === 'fund' ? null : Validators.required); + });src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts (1)
Line range hint
260-265
: Remove unnecessary else clauses to simplify control flow.- } else if (this.amountFormat > +amountCheck) { + if (this.amountFormat > +amountCheck) {src/app/pages/token-cosmos/nft-detail/nft-detail.component.ts (1)
Line range hint
226-226
: Simplify the conditional expression ingetDataTable
.- const isCW4973 = this.isCW4973 ? true : false; + const isCW4973 = !!this.isCW4973;This change makes the code cleaner and more concise.
src/app/pages/validators/validators.component.ts (1)
Line range hint
360-360
: Consider using optional chaining for safer property access.Using optional chaining (
?.
) can prevent runtime errors when accessing properties on objects that might benull
orundefined
. This is a minor change but can improve the robustness of your code.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
src/assets/fonts/Space_Mono/SpaceMono-Bold.ttf
is excluded by!**/*.ttf
src/assets/fonts/Space_Mono/SpaceMono-BoldItalic.ttf
is excluded by!**/*.ttf
src/assets/fonts/Space_Mono/SpaceMono-Italic.ttf
is excluded by!**/*.ttf
src/assets/fonts/Space_Mono/SpaceMono-Regular.ttf
is excluded by!**/*.ttf
Files selected for processing (28)
- src/app/core/constants/token.constant.ts (1 hunks)
- src/app/core/pipes/custom-pipe.module.ts (3 hunks)
- src/app/core/pipes/highlight-function.pipe.ts (1 hunks)
- src/app/core/services/token.service.ts (5 hunks)
- src/app/core/services/wallet.service.ts (6 hunks)
- src/app/core/utils/ethers/utils.ts (1 hunks)
- src/app/core/utils/ethers/validate.ts (1 hunks)
- src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract-content.component.ts (1 hunks)
- src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-read/evm-read.component.ts (5 hunks)
- src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-write/evm-write.component.ts (2 hunks)
- src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.html (1 hunks)
- src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.ts (3 hunks)
- src/app/pages/evm-token/evm-token-summary/evm-token-summary.component.html (1 hunks)
- src/app/pages/token-cosmos/nft-detail/nft-detail.component.ts (1 hunks)
- src/app/pages/tokens/account-bound-tokens/account-bound-tokens.component.html (1 hunks)
- src/app/pages/tokens/non-fungible-tokens/non-fungible-tokens.component.html (1 hunks)
- src/app/pages/tokens/non-fungible-tokens/non-fungible-tokens.component.ts (1 hunks)
- src/app/pages/transaction/evm-transaction/evm-message/evm-message.component.html (7 hunks)
- src/app/pages/transaction/evm-transaction/evm-message/evm-message.component.scss (1 hunks)
- src/app/pages/transaction/evm-transaction/evm-transaction-event-log/evm-transaction-event-log.component.html (2 hunks)
- src/app/pages/transaction/evm-transaction/evm-transaction-event-log/evm-transaction-event-log.component.scss (1 hunks)
- src/app/pages/transaction/evm-transaction/evm-transaction-event-log/evm-transaction-event-log.component.ts (1 hunks)
- src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts (2 hunks)
- src/app/pages/validators/validators.component.ts (5 hunks)
- src/assets/fonts/Space_Mono/Space_Mono.css (1 hunks)
- src/assets/scss/aura-theme/variables.scss (1 hunks)
- src/assets/scss/custom.scss (1 hunks)
- src/styles.scss (1 hunks)
Files skipped from review due to trivial changes (9)
- src/app/core/constants/token.constant.ts
- src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.html
- src/app/pages/tokens/non-fungible-tokens/non-fungible-tokens.component.html
- src/app/pages/transaction/evm-transaction/evm-message/evm-message.component.html
- src/app/pages/transaction/evm-transaction/evm-message/evm-message.component.scss
- src/app/pages/transaction/evm-transaction/evm-transaction-event-log/evm-transaction-event-log.component.scss
- src/app/pages/transaction/evm-transaction/evm-transaction-event-log/evm-transaction-event-log.component.ts
- src/assets/fonts/Space_Mono/Space_Mono.css
- src/styles.scss
Additional context used
Biome
src/app/core/utils/ethers/validate.ts
[error] 12-12: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression withsrc/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-read/evm-read.component.ts
[error] 56-56: Avoid the use of spread (
...
) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-write/evm-write.component.ts
[error] 74-74: Avoid the use of spread (
...
) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts
[error] 92-92: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 260-265: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 263-265: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
[error] 201-201: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 229-229: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
src/app/pages/token-cosmos/nft-detail/nft-detail.component.ts
[error] 226-226: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression withsrc/app/pages/validators/validators.component.ts
[error] 360-360: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 538-538: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 567-567: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 663-663: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 690-690: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 727-727: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 757-757: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
Additional comments not posted (18)
src/app/core/utils/ethers/validate.ts (2)
7-7
: LGTM! Wrapping the value in an object ensures consistency in the function's return type.
14-14
: Consistent handling of default cases by wrapping the value in an object.src/app/core/utils/ethers/utils.ts (1)
34-41
: Excellent addition of error handling in theaddNetwork
function. This ensures robustness by catching and logging errors during network configuration changes.src/app/core/pipes/highlight-function.pipe.ts (1)
1-31
: Well-implementedHighlightFunctionPipe
that enhances the readability of function signatures by styling individual components. Good use ofDomSanitizer
to ensure safe rendering of HTML.src/assets/scss/aura-theme/variables.scss (1)
74-74
: Good addition of the--font-space-mono
variable, enhancing the theming flexibility of the application.src/app/core/pipes/custom-pipe.module.ts (2)
16-16
: The import ofHighlightFunctionPipe
is correctly added.
44-44
:HighlightFunctionPipe
is correctly declared and exported in theNgModule
decorator.Also applies to: 70-70
src/app/pages/transaction/evm-transaction/evm-transaction-event-log/evm-transaction-event-log.component.html (1)
15-15
: The addition of thefont-space-mono
class and the use of thehighlight_function
pipe enhance the readability and styling of the event log.Also applies to: 27-27, 29-30, 33-35, 49-49
src/app/pages/evm-token/evm-token-summary/evm-token-summary.component.html (1)
37-37
: The addition of the[iconContract]
attribute correctly enhances the display of contract icons in the token summary.src/app/pages/tokens/account-bound-tokens/account-bound-tokens.component.html (1)
66-66
: The addition of the[iconContract]
attribute correctly enhances the display of contract icons in the account-bound tokens component.src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract/evm-read/evm-read.component.ts (1)
42-42
: Good addition of dependency injection forWalletService
.This change integrates wallet functionalities directly into the component, enhancing its capabilities.
src/app/pages/evm-contracts/evm-contracts-detail/evm-contract-content/evm-contract-content.component.ts (1)
170-170
: Optimize object cloning to enhance performance.Using the spread operator in this context can lead to performance issues. Consider using
Object.assign
for a more efficient deep copy.- this.contractTransaction = {...this.contractTransaction}; + Object.assign(this.contractTransaction, {data: res, count: this.contractTransaction['data'].length || 0});Likely invalid or redundant comment.
src/assets/scss/custom.scss (1)
463-467
: LGTM! The addition of the.font-space-mono
class enhances the typographic options available for monospace text.src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.ts (2)
38-38
: Import ofTOKEN_EVM_BURNT
constant is correctly added and used in the component.
159-159
: UsingTOKEN_EVM_BURNT
for the ownership check enhances maintainability and readability by replacing the hardcoded address.src/app/core/services/token.service.ts (3)
11-11
: Import ofTOKEN_EVM_BURNT
constant is correctly added and used in the service.
Line range hint
275-286
: The modification to exclude tokens owned by theTOKEN_EVM_BURNT
address in the GraphQL query is a good practice to filter out irrelevant data.
338-338
: The modifications to the GraphQL queries to exclude tokens owned by theTOKEN_EVM_BURNT
address are correctly implemented, enhancing data relevance and consistency.Also applies to: 427-435
async createContract(contractAddr, evmAccount) { | ||
try { | ||
const connected = await this.walletService.connectToChain(); | ||
if (!connected) { | ||
this.isLoading = false; | ||
this.isHandleStake = false; | ||
this.toastr.error(`Please switch to ${this.environmentService.evmChainInfo.chain} chain.`); | ||
return null; | ||
} | ||
|
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.
Consider handling the case when connectToChain
fails more gracefully.
The current implementation logs an error and shows a toast message but does not throw or handle the error in a way that could be used by the calling function. Consider throwing an error or using a different approach to handle this case more robustly.
@@ -547,7 +555,7 @@ export class ValidatorsComponent implements OnInit, OnDestroy { | |||
this.checkTxStatusOnchain({ error }); | |||
}); | |||
} else { | |||
const contract = this.createContract(this.stakeContractAddr, account.evmAccount); | |||
const contract = await this.createContract(this.stakeContractAddr, account.evmAccount); |
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.
Use the exponentiation operator for better readability.
Replace Math.pow
with the exponentiation operator (**
). This change will make the code more modern and easier to read.
- (this.amountFormat * Math.pow(10, 18)).toFixed(0),
+ (this.amountFormat * 10**18).toFixed(0),
Repeat this change for all instances of Math.pow
in the file.
Also applies to: 567-567, 663-663, 690-690, 727-727, 757-757
Committable suggestion was skipped due to low confidence.
async createContract(contractAddr, evmAccount) { | ||
try { | ||
const connected = await this.walletService.connectToChain(); | ||
if (!connected) { | ||
return null; | ||
} |
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.
Ensure proper error handling when the connection fails in the createContract
method.
Consider adding a user-friendly error message or logging when the connection fails.
@@ -213,7 +217,7 @@ export class DelegateItemComponent implements OnInit { | |||
this.checkTxStatusOnchain({ error }); | |||
}); | |||
} else { | |||
const contract = this.createContract(this.stakeContractAddr, account.evmAccount); | |||
const contract = await this.createContract(this.stakeContractAddr, account.evmAccount); |
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.
Use the exponentiation operator for clarity and modern JavaScript practices.
- const amount = (this.amountFormat * Math.pow(10, 18)).toFixed(0);
+ const amount = (this.amountFormat * 10**18).toFixed(0);
Also applies to: 229-229
Committable suggestion was skipped due to low confidence.
if (value?.toLowerCase() !== 'true' && value?.toLowerCase() !== 'false') { | ||
return { value, error: `"${value}" is an invalid parameter, please use true/false.` }; | ||
} | ||
return { value: value?.toLowerCase() === 'true' ? true : false }; |
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.
Great improvement in type safety for boolean values. However, consider simplifying the return statement.
- return { value: value?.toLowerCase() === 'true' ? true : false };
+ return { value: value?.toLowerCase() === 'true' };
This change removes the unnecessary ternary operation, making the code cleaner and more efficient.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (value?.toLowerCase() !== 'true' && value?.toLowerCase() !== 'false') { | |
return { value, error: `"${value}" is an invalid parameter, please use true/false.` }; | |
} | |
return { value: value?.toLowerCase() === 'true' ? true : false }; | |
if (value?.toLowerCase() !== 'true' && value?.toLowerCase() !== 'false') { | |
return { value, error: `"${value}" is an invalid parameter, please use true/false.` }; | |
} | |
return { value: value?.toLowerCase() === 'true' }; |
Tools
Biome
[error] 12-12: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
evmChangeEvent() { | ||
const reconnect = () => { | ||
const timeoutId = setTimeout(() => { | ||
clearTimeout(timeoutId); | ||
this.connectToChain(); | ||
}, 1000); | ||
}; | ||
(window as any).ethereum?.on('accountsChanged', reconnect); | ||
(window as any).ethereum?.on('chainChanged', reconnect); | ||
} |
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.
Optimize the event handling in evmChangeEvent
to avoid potential memory leaks.
Consider removing the event listener when it's no longer needed to prevent memory leaks.
const connected = await this.connectToChain(); | ||
if (!connected) { | ||
return; |
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.
Ensure proper error handling when the connection fails in connectEvmWallet
.
Consider adding a user-friendly error message or logging when the connection fails.
let tagAddress = `["${keySearch}"]`; | ||
if (listAddress?.length > 0) { | ||
tagAddress = ''; | ||
listAddress.forEach((addr) => { | ||
tagAddress += `"${addr}",`; | ||
}); | ||
tagAddress = `[${tagAddress}]`; | ||
tagAddress = `[${tagAddress}"${keySearch}"]`; |
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.
Refactor address handling in getTokenData
.
The current implementation concatenates strings in a way that could lead to errors or inefficiencies. Consider using array methods for cleaner and more efficient code.
- let tagAddress = `["${keySearch}"]`;
- if (listAddress?.length > 0) {
- tagAddress = '';
- listAddress.forEach((addr) => {
- tagAddress += `"${addr}",`;
- });
- tagAddress = `[${tagAddress}"${keySearch}"]`;
- }
+ const addresses = listAddress.length > 0 ? listAddress : [keySearch];
+ const tagAddress = JSON.stringify(addresses);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let tagAddress = `["${keySearch}"]`; | |
if (listAddress?.length > 0) { | |
tagAddress = ''; | |
listAddress.forEach((addr) => { | |
tagAddress += `"${addr}",`; | |
}); | |
tagAddress = `[${tagAddress}]`; | |
tagAddress = `[${tagAddress}"${keySearch}"]`; | |
const addresses = listAddress.length > 0 ? listAddress : [keySearch]; | |
const tagAddress = JSON.stringify(addresses); |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/core/services/token.service.ts (8 hunks)
Additional comments not posted (5)
src/app/core/services/token.service.ts (5)
11-11
: Introduced import forTOKEN_EVM_BURNT
. This aligns with the PR's objective to handle burnt tokens more effectively.
Line range hint
275-286
: Updated query conditions to exclude burnt tokens usingTOKEN_EVM_BURNT
. This enhances the accuracy of token listings by ensuring burnt tokens are not included.
338-338
: Updated query condition to exclude burnt tokens in token count. This ensures more accurate analytics and data integrity.
427-435
: Correctly updated query conditions to exclude burnt tokens in ERC721 token holder listings. This adjustment is crucial for maintaining accurate and meaningful data.
Line range hint
556-607
: Introduced filtering for specific actions in ERC721 transfers. Ensure that the actions included inqueryActionIn
are relevant and necessary for the intended operations.Verification successful
The search results show that
queryActionIn
is defined as['transfer']
and used in the same file. This indicates that the filtering is specifically for the 'transfer' action in ERC721 transfers.
- src/app/core/services/token.service.ts:
const queryActionIn = ['transfer'];
- src/app/core/services/token.service.ts:
actionIn: queryActionIn,
This confirms that the action included in
queryActionIn
is relevant to ERC721 transfers.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the relevance of actions included in ERC721 transfers. # Test: Search for usage of `queryActionIn` in ERC721 transfer queries. Expect: Relevant actions only. rg --type typescript $'queryActionIn'Length of output: 71
Script:
#!/bin/bash # Description: Verify the relevance of actions included in ERC721 transfers. # Test: Search for usage of `queryActionIn` in ERC721 transfer queries. Expect: Relevant actions only. rg 'queryActionIn'Length of output: 171
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts (1)
Line range hint
263-268
: This else clause can be omitted because previous branches break early.isValidAmount(): void { let amountCheck = this.validatorService.getMaxAmount( this.dataDelegate.availableToken, this.dataDelegate.delegableVesting, ); this.isExceedAmount = false; this.isValidatorJail = false; if (this.currentValidatorDetail?.jailed !== 0) { this.isValidatorJail = true; return; } else if (this.amountFormat > +amountCheck) { this.isExceedAmount = true; return; } this.isHandleStake = true; // This line can be moved outside the else block }You can simplify the function by removing the else clause and placing the last line outside of it, as the function returns early in all other cases.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts (2 hunks)
Additional context used
Biome
src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts
[error] 92-92: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 263-268: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 266-268: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
[error] 204-204: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
[error] 232-232: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)
Unsafe fix: Use the '**' operator instead of 'Math.pow'.
Additional comments not posted (1)
src/app/pages/validators/validators-detail/delegate-item/delegate-item.component.ts (1)
223-223
: Use the exponentiation operator for clarity and modern JavaScript practices.- const amount = (this.amountFormat * Math.pow(10, 18)).toFixed(0); + const amount = (this.amountFormat * 10**18).toFixed(0);Also applies to line 232.
Also applies to: 232-232
Likely invalid or redundant comment.
async createContract(contractAddr, evmAccount) { | ||
try { | ||
const connected = await this.walletService.connectToChain(); | ||
if (!connected) { | ||
this.isLoading = false; | ||
this.isHandleStake = false; | ||
this.toastr.error(`Please switch to ${this.environmentService.evmChainInfo.chain} chain.`); | ||
return null; | ||
} |
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.
Ensure proper error handling when the connection fails in the createContract
method.
Consider adding a user-friendly error message or logging when the connection fails, as previously suggested. Here's an updated version of the method with enhanced error handling:
async createContract(contractAddr, evmAccount) {
try {
const connected = await this.walletService.connectToChain();
if (!connected) {
this.isLoading = false;
this.isHandleStake = false;
this.toastr.error(`Please switch to ${this.environmentService.evmChainInfo.chain} chain.`);
throw new Error('Connection failed'); // Added to throw an error on connection failure
}
let contract = new Contract(contractAddr, stakeAbi, evmAccount);
if (contract) {
this.contract = contract;
return this.contract;
}
} catch (error) {
console.error('Failed to create contract:', error);
throw error; // Ensure that errors are propagated
}
return null;
}
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/app/core/services/token.service.ts (5 hunks)
- src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.ts (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/app/core/services/token.service.ts
Additional comments not posted (2)
src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.ts (2)
38-38
: Import ofTOKEN_EVM_BURNT
constant added.This import is necessary for the new logic to determine if a token is burned. Good practice to keep constants centralized.
159-159
: Logic to determine if a token is burned.The use of the
TOKEN_EVM_BURNT
constant to check theowner
field is a clean and clear way to determine the burned status of a token.
@@ -51,7 +52,7 @@ export class EvmNFTDetailComponent implements OnInit { | |||
templates: Array<TableTemplate> = [ | |||
{ matColumnDef: 'tx_hash', headerCellDef: 'Txn Hash' }, | |||
{ matColumnDef: 'type', headerCellDef: 'Method' }, | |||
{ matColumnDef: 'status', headerCellDef: 'Result' }, | |||
// { matColumnDef: 'status', headerCellDef: 'Result' }, |
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.
Commented out the 'status' column in the table.
This change might affect the UI by removing important information about transaction results. Please confirm if this change aligns with the intended user experience.
return erc721Activities.map((tx, idx) => { | ||
const methodId = _.get(tx, 'evm_transaction.data')?.substring(0, 8); | ||
let methodType = mappingMethodName(element, methodId); | ||
|
||
if (methodType === 'Burn') { | ||
if (idx > 0 && erc721Activities[idx - 1].tempType === 'Burn') { | ||
methodType = 'Approval'; | ||
} else { | ||
methodType = 'Transfer'; | ||
tx.tempType = 'Burn'; | ||
} | ||
} | ||
return { | ||
...tx, | ||
type: mappingMethodName(element, methodId), | ||
type: methodType, |
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.
Complex logic in getDataTable
method for mapping transaction types.
This method is quite complex and handles multiple responsibilities. Consider refactoring to improve readability and maintainability. For instance, breaking down this method into smaller, more focused methods could help.
- if (methodType === 'Burn') {
- if (idx > 0 && erc721Activities[idx - 1].tempType === 'Burn') {
- methodType = 'Approval';
- } else {
- methodType = 'Transfer';
- tx.tempType = 'Burn';
- }
- }
+ methodType = this.resolveMethodType(methodType, idx, erc721Activities);
And then define resolveMethodType
method to encapsulate the conditional logic.
Committable suggestion was skipped due to low confidence.
This reverts commit ff8b07f.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/app/pages/evm-token/evm-nft-detail/evm-nft-detail.component.ts
Summary by CodeRabbit
New Features
HighlightFunctionPipe
for transforming and styling function signatures.[iconContract]
attribute to various components for displaying contract icons.WalletService
with methods for handling Ethereum chain and account changes.Bug Fixes
EvmWriteComponent
andDelegateItemComponent
.TOKEN_EVM_BURNT
.Improvements
TokenService
to filter out burnt tokens.