Polyfill #84
Polyfill #84
Conversation
WalkthroughThe pull request includes multiple changes across various files, primarily focusing on code formatting, dependency management, and the removal of certain functionalities. Key modifications involve the removal of semicolons from TypeScript import statements and function calls for consistency, updates to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
💤 Files with no reviewable changes (8)
🚧 Files skipped from review as they are similar to previous changes (2)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Dependency ReviewThe following issues were found:
OpenSSF Scorecard
Scanned Manifest Filespackage-lock.json
package.json
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (43)
src/client/lcd/api/ForwardingAPI.ts (1)
42-43: LGTM! Minor improvement in parseInt usage.The change from
parseInttoNumber.parseIntis a slight improvement in code clarity. Both functions behave identically in modern JavaScript/TypeScript environments, butNumber.parseIntis more explicit about being a method of theNumberobject. This change maintains the existing functionality while potentially improving code readability.Consider adding a radix parameter (e.g.,
Number.parseInt(d.num_of_accounts, 10)) to explicitly specify base 10, which is a good practice to ensure consistent behavior across all environments.src/core/celestia/BlobTx.ts (1)
45-47: LGTM! Consider updating the constructor parameter name for consistency.The changes in the
fromProtomethod look good and align with the PR objectives:
- Using
Tx.fromBytes(data.tx)instead ofTx.fromBuffer(Buffer.from(data.tx))is consistent with the goal of replacing Buffer with Uint8Array.- The change from
type_idtotypeIdin the parameter reference improves naming consistency.For complete consistency, consider updating the constructor parameter name from
type_idtotypeId. This would align with the naming convention used in thefromProtomethod. Here's the suggested change:constructor( public tx: Tx, public blobs: Blob[], - public type_id: string + public typeId: string ) { super() }Don't forget to update any references to
this.type_idin other methods of the class if you make this change.src/client/lcd/api/TendermintAPI.spec.ts (1)
15-15: LGTM: Replaced Buffer.from with bytesFromBase64The
bytesFromBase64function is correctly used to decode the transaction bytes, replacing the previousBuffer.frommethod. This change aligns with the PR objective of replacing Buffer with Uint8Array for better compatibility.Consider adding a comment explaining the purpose of this decoding step for improved readability:
// Decode base64-encoded transaction bytes to Uint8Array const txProto = Tx_pb.decode(bytesFromBase64(txBytes))src/client/lcd/api/OpchildAPI.ts (1)
Update Remaining
parseIntInstances toNumber.parseIntWhile many instances have been updated to use
Number.parseInt, there are still some occurrences of the globalparseIntfunction, such as insrc/util/polyfill.ts. For consistency and to avoid potential scope issues, please update all remaining instances toNumber.parseInt.
src/util/polyfill.ts: (hex.match(/.{1,2}/g) ?? []).map((byte) => parseInt(byte, 16))🔗 Analysis chain
Line range hint
1-74: Consider updating all instances ofparseIntin the codebaseThe changes made to use
Number.parseIntinstead ofparseIntin this file are good improvements. For consistency, it might be beneficial to update all instances ofparseIntacross the entire codebase to useNumber.parseInt.To identify other instances of
parseIntusage in the codebase, you can run the following command:This will help ensure consistent usage of
Number.parseIntthroughout the project.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining instances of parseInt in TypeScript files rg 'parseInt\(' --type tsLength of output: 19835
src/core/ibc/core/client/Height.ts (1)
36-37: Approve the change toNumber.parseIntwith a minor suggestion.The switch from
parseInttoNumber.parseIntis a good change. It makes the code more explicit and less prone to confusion with the globalparseIntfunction.For consistency, consider applying this change throughout the codebase, including in the
fromDatamethod of this class.Here's a suggestion for the
fromDatamethod:public static fromData(data: Height.Data): Height { const { revision_number, revision_height } = data return new Height( - parseInt(revision_number), - parseInt(revision_height) + Number.parseInt(revision_number), + Number.parseInt(revision_height) ) }src/core/ophost/msgs/MsgRecordBatch.ts (2)
72-72: Approved: Effective use of polyfill functionThe replacement of
Buffer.from(data.batchBytes).toString('base64')withbase64FromBytes(data.batchBytes)is an excellent change. It aligns with the PR objective and simplifies the code.Consider adding a null check for
data.batchBytesto handle potential undefined values gracefully:base64FromBytes(data.batchBytes ?? new Uint8Array())This ensures that the function always receives a valid input, even if
data.batchBytesis undefined.
81-81: Approved: Consistent use of polyfill function with null checkThe replacement of
Buffer.from(batch_bytes, 'base64')withbytesFromBase64(batch_bytes)is excellent. The null check is a good practice to handle potential undefined values.For consistency with the
fromProtomethod, consider using a default emptyUint8Array:batchBytes: bytesFromBase64(batch_bytes ?? ''),This approach ensures that
batchBytesis always aUint8Array, maintaining consistency with thefromProtomethod and potentially simplifying type handling in the proto message.src/core/tx/SignDoc.ts (2)
67-68: LGTM: Successful replacement of Buffer with base64FromBytesThe changes in the
toDatamethod successfully replace the Node.js-specificBufferusage with the more compatiblebase64FromBytesfunction. This modification aligns well with the PR's objective and improves cross-platform compatibility.For consistency, consider using template literals for the entire return object:
return { body_bytes: base64FromBytes(tx_body.toBytes()), auth_info_bytes: base64FromBytes(auth_info.toBytes()), account_number: `${account_number}`, chain_id, };This change would make the
account_numberconversion consistent with the new base64 conversions, using template literals instead of thetoFixed()method.
Buffer usage detected in multiple TypeScript files
The verification identified remaining instances of
Bufferusage in the following files:
src/util/polyfill.tssrc/key/ledger/app.tssrc/key/ledger/device.tssrc/key/ledger/LedgerKey.tsPlease review these instances to ensure they align with the PR's objective of removing Node.js-specific dependencies and enhancing cross-platform compatibility.
🔗 Analysis chain
Line range hint
1-124: Overall assessment: Changes successfully implement PR objectivesThe modifications in this file successfully replace the Node.js-specific
Bufferusage with the more compatiblebase64FromBytesfunction. These changes align well with the PR's objective of enhancing compatibility and maintain the existing functionality of theSignDocclass. The implementation is clean and consistent with the rest of the codebase.To ensure that all
Bufferusages have been replaced in the codebase, run the following verification script:This script will help identify any remaining
Bufferusages that may need to be addressed in other parts of the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining Buffer usages in the codebase # Test: Search for Buffer usages. Expect: No results or only in comments/strings rg --type typescript 'Buffer\.' -C 3Length of output: 73
Script:
#!/bin/bash # Description: Check for any remaining Buffer usages in TypeScript files # Test: Search for Buffer usages in .ts files. Expect: No results or only in comments/strings rg 'Buffer\.' -g '*.ts' -C 3Length of output: 7931
src/core/wasm/msgs/MsgExecuteContract.ts (2)
71-71: LGTM: Appropriate use of polyfill functionThe replacement of
Buffer.from(data.msg).toString('base64')withbase64FromBytes(data.msg)is a good change. It aligns with the PR objective and should improve cross-platform compatibility.Consider adding error handling to account for potential invalid input:
base64FromBytes(data.msg ?? new Uint8Array())This ensures that even if
data.msgis undefined or null, the function won't throw an error.
81-81: LGTM: Appropriate use of polyfill functionThe replacement of
Buffer.from(msg, 'base64')withbytesFromBase64(msg)is a good change. It aligns with the PR objective and should improve cross-platform compatibility.Consider adding error handling to account for potential invalid input:
bytesFromBase64(msg || '')This ensures that even if
msgis undefined or null, the function won't throw an error.src/core/ibc/core/channel/msgs/MsgChannelOpenConfirm.ts (1)
Incomplete Buffer Replacement Detected
The verification process identified multiple instances of
Bufferusage that were not addressed in the recent changes. To maintain consistency and achieve the PR's objectives, please ensure that allBufferusages are replaced accordingly.Affected Files:
src/util/polyfill.tssrc/key/ledger/app.tssrc/key/ledger/device.ts- Numerous files within
src/core/and its subdirectories.🔗 Analysis chain
Line range hint
1-114: Summary: Successful implementation of Buffer replacementThe changes in this file consistently replace Buffer usage with new utility functions for base64 encoding and decoding. This aligns well with the PR objective of enhancing compatibility and performance. The existing interface and functionality are maintained while improving the internal handling of base64 data.
To ensure consistency across the project, it would be beneficial to verify similar changes in other files. You can use the following script to check for any remaining Buffer usages:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for remaining Buffer usages in TypeScript files # Search for Buffer usages in TypeScript files echo "Searching for remaining Buffer usages:" rg --type typescript 'Buffer\.' -g '!node_modules' # Search for files that might need similar changes echo "Files that might need similar changes:" rg --type typescript -l 'base64' -g '!node_modules'Length of output: 348
Script:
#!/bin/bash # Description: Check for remaining Buffer usages in TypeScript files # Search for Buffer usages in .ts files echo "Searching for remaining Buffer usages:" rg 'Buffer\.' -g '*.ts' --ignore-dir node_modules # Search for files that might need similar changes by looking for 'base64' echo "Files that might need similar changes:" rg 'base64' -g '*.ts' --ignore-dir node_modulesLength of output: 1159
Script:
#!/bin/bash # Description: Check for remaining Buffer usages in TypeScript files # Search for Buffer usages in .ts files, excluding node_modules echo "Searching for remaining Buffer usages:" rg 'Buffer\.' -g '*.ts' -g '!node_modules/**' # Search for files that might need similar changes by looking for 'base64', excluding node_modules echo "Files that might need similar changes:" rg 'base64' -g '*.ts' -g '!node_modules/**'Length of output: 18555
src/core/wasm/msgs/MsgMigrateContract.ts (2)
80-80: LGTM: Successful replacement of Buffer with base64FromBytes.The change effectively replaces the Buffer usage with the new
base64FromBytesutility function, aligning with the PR objectives. This should improve cross-platform compatibility.For improved clarity, consider adding a comment explaining the purpose of this conversion:
// Convert binary message data to base64 string base64FromBytes(data.msg)
90-90: LGTM: Successful replacement of Buffer with bytesFromBase64.The change effectively replaces the Buffer usage with the new
bytesFromBase64utility function, aligning with the PR objectives. This should improve cross-platform compatibility.For improved clarity, consider adding a comment explaining the purpose of this conversion:
// Convert base64 string message to binary data msg: bytesFromBase64(msg),src/core/ibc/core/channel/msgs/MsgRecvAcknowledgement.ts (1)
79-80: LGTM: Replaced Buffer usage with bytesFromBase64 utilityThe changes in the
toProtomethod effectively replace the use ofBufferwith thebytesFromBase64utility function. This modification aligns with the PR objective and enhances cross-platform compatibility while maintaining the method's functionality.Consider updating the property name
proof_ackedtoproofAckedfor consistency with the proto object property name. This would improve code readability and maintain a consistent naming convention throughout the class.src/core/bech32.spec.ts (1)
25-25: LGTM: Replaced Buffer with bytesFromUtf8.The change from
Buffer.from('foobar', 'utf8')tobytesFromUtf8('foobar')is correct and aligns with the PR objective. This modification enhances cross-platform compatibility.Consider extracting the string 'foobar' into a constant at the top of the file, as it's used multiple times in the tests. This would improve maintainability:
const TEST_STRING = 'foobar'; // Then use it like this: const words = bech32.toWords(bytesFromUtf8(TEST_STRING));src/core/ibc/core/client/msgs/tendermint/crypto.ts (2)
14-21: LGTM: Consistent naming convention applied.The renaming of
leafHashtoleaf_hashimproves consistency with snake_case naming conventions often used in protobuf definitions. This change is correctly reflected in both the constructor parameter and the public property declaration.Consider updating the JSDoc comment to reflect the new parameter name:
/** * @param total * @param index - * @param leaf_hash + * @param leaf_hash * @param aunts */
130-131: LGTM: Consistent use of new utility functions for base64 conversions.The changes in the
PublicKeyclass methods (fromProtoandtoProto) are consistent with the modifications made in theProofclass. The use ofbase64FromBytesandbytesFromBase64for convertinged25519andsecp256k1aligns with the PR objective of replacing Buffer with Uint8Array.For consistency with the
Proofclass implementation, consider using the nullish coalescing operator (??) instead of the ternary operator:- ed25519: ed25519 ? bytesFromBase64(ed25519) : undefined, - secp256k1: secp256k1 ? bytesFromBase64(secp256k1) : undefined, + ed25519: bytesFromBase64(ed25519 ?? ''), + secp256k1: bytesFromBase64(secp256k1 ?? ''),This change would make the code more concise and consistent with the
Proofclass implementation.Also applies to: 138-139
src/util/polyfill.spec.ts (2)
43-81: LGTM: Hex tests are well-structured and comprehensive.The hex tests cover essential scenarios:
- Converting hex string to Uint8Array
- Handling empty hex strings
- Converting Uint8Array to hex string
- Handling empty Uint8Array
The test data and expected results are correct, ensuring robust coverage of the hex utility functions.
Consider updating the comment on line 65 to remove the reference to Buffer:
- // Converts a hex string to a Uint8Array using Buffer when available + // Converts a Uint8Array to a hex string
83-121: LGTM: UTF-8 tests are well-structured and comprehensive.The UTF-8 tests cover essential scenarios:
- Converting UTF-8 string to Uint8Array
- Handling empty UTF-8 strings
- Converting Uint8Array to UTF-8 string
- Handling empty Uint8Array
The test data and expected results are correct, ensuring robust coverage of the UTF-8 utility functions.
Consider updating the comment on line 105 to remove the reference to Buffer:
- // Converts a utf8 string to a Uint8Array using Buffer when available + // Converts a Uint8Array to a UTF-8 stringsrc/core/ibc/core/channel/msgs/MsgChannelOpenAck.ts (1)
Line range hint
1-150: Overall assessment: Changes look good and align with PR objectives.The modifications in this file consistently replace Buffer usage with the new utility functions
base64FromBytesandbytesFromBase64. These changes should improve compatibility across different JavaScript environments and potentially enhance performance, as intended by the PR objectives.To ensure full compatibility, consider adding unit tests that cover these changes, especially focusing on edge cases for base64 encoding and decoding.
Would you like assistance in generating unit tests for the updated
MsgChannelOpenAckclass?src/core/ophost/msgs/MsgProposeOutput.ts (1)
95-95: LGTM with a minor suggestion: Use of base64FromBytes in fromProto methodThe replacement of
Buffer.from(data.outputRoot).toString('base64')withbase64FromBytes(data.outputRoot)is consistent with the PR objective and improves cross-environment compatibility.Consider handling the case where
data.outputRootmight be undefined:base64FromBytes(data.outputRoot ?? new Uint8Array())This ensures that the function always receives a valid input, even if
data.outputRootis undefined.src/core/ibc/core/channel/msgs/MsgChannelOpenTry.ts (3)
95-95: LGTM: Replaced Buffer usage with polyfill utility.The change from
Buffer.from(proto.proofInit).toString('base64')tobase64FromBytes(proto.proofInit)successfully replaces the Buffer usage with the new polyfill utility function. This aligns with the PR objective and maintains the same functionality.Consider adding a comment explaining the purpose of this conversion for better code readability:
// Convert proof_init from Uint8Array to base64 string base64FromBytes(proto.proofInit),
116-116: LGTM: Replaced Buffer usage with polyfill utility.The change from
Buffer.from(proof_init, 'base64')tobytesFromBase64(proof_init)successfully replaces the Buffer usage with the new polyfill utility function. This aligns with the PR objective and maintains the same functionality.Consider adding a comment explaining the purpose of this conversion for better code readability:
// Convert proof_init from base64 string to Uint8Array proofInit: bytesFromBase64(proof_init),
Line range hint
1-164: Overall implementation looks good.The changes in this file consistently replace Buffer usage with polyfill utilities, aligning well with the PR objectives. The modifications improve compatibility and potentially enhance performance without altering the core functionality. The implementation is correct and consistent throughout the file.
Consider adding a brief comment at the top of the file explaining the transition from Buffer to Uint8Array and the use of polyfill utilities. This would provide context for future developers working on this code.
src/core/move/msgs/MsgExecute.ts (2)
87-87: Approved: Consistent use of polyfill functionThe change from
Buffer.from(arg).toString('base64')tobase64FromBytesis consistent with the PR objective and the new import. This modification enhances compatibility by removing the dependency on Node.js Buffer.For improved clarity, consider adding a comment explaining the purpose of this transformation:
data.args.map(base64FromBytes) // Convert byte arrays to base64 strings
106-106: Approved: Consistent use of polyfill functionThe change from
args.map((arg) => Buffer.from(arg, 'base64'))toargs.map(bytesFromBase64)is consistent with the PR objective and the new import. This modification enhances compatibility by removing the dependency on Node.js Buffer.For improved clarity, consider adding a comment explaining the purpose of this transformation:
args: args.map(bytesFromBase64), // Convert base64 strings to byte arrayssrc/core/wasm/msgs/MsgInstantiateContract.ts (1)
114-114: LGTM: Improved base64 encoding intoProtomethodThe use of
bytesFromBase64(msg)simplifies the encoding process and aligns with the PR objective of replacing Buffer operations. This change enhances code consistency with thefromProtomethod.For consistency with the class property name, consider renaming the destructured variable:
-const { sender, admin, code_id, label, msg, funds } = this +const { sender, admin, code_id, label, msg: msgBase64, funds } = thisThen use
msgBase64in thebytesFromBase64call:-msg: bytesFromBase64(msg), +msg: bytesFromBase64(msgBase64),This change would make it clearer that we're working with a base64-encoded string.
src/core/wasm/msgs/MsgStoreAndMigrateContract.ts (2)
2-2: LGTM! Consider grouping related imports.The addition of
base64FromBytesandbytesFromBase64imports aligns with the PR objective of replacing Buffer with Uint8Array for improved compatibility and performance.Consider grouping related imports together. You could move this import statement next to other utility imports for better organization.
Line range hint
1-164: Overall assessment: Changes align with PR objectives, but clarification needed.The modifications in this file successfully replace Buffer usage with new utility functions for base64 encoding/decoding, aligning with the PR's goal of enhancing compatibility and potentially improving performance. The changes are applied consistently throughout the file.
However, there's a discrepancy between the AI-generated summary and the visible changes regarding method signature updates for
fromProtoandtoProto. This inconsistency needs to be addressed to ensure accuracy in the PR description and to verify if any additional changes are required.Please review the comments above, address the suggestions, and clarify the status of the method signature changes mentioned in the AI summary.
src/core/wasm/msgs/MsgInstantiateContractV2.ts (2)
114-116: LGTM: Successful replacement of Buffer with base64FromBytesThe changes in the
fromProtomethod successfully replace the Buffer usage with the newbase64FromBytesfunction, aligning with the PR objective. This maintains the same functionality while improving compatibility.Consider adding a comment explaining the base64 conversion for future maintainers:
// Convert binary data to base64 string base64FromBytes(data.msg), // ... base64FromBytes(data.salt),
128-130: LGTM: Successful replacement of Buffer with bytesFromBase64The changes in the
toProtomethod successfully replace the Buffer usage with the newbytesFromBase64function, aligning with the PR objective. This maintains the same functionality while improving compatibility.Consider adding a comment explaining the base64 conversion for future maintainers:
// Convert base64 string to binary data msg: bytesFromBase64(msg), // ... salt: bytesFromBase64(salt),src/client/lcd/api/OphostAPI.ts (1)
167-167: LGTM: Improved parsing method, consider specifying radix.The change from
parseInt(d.next_l1_sequence)toNumber.parseInt(d.next_l1_sequence)is a good improvement. It uses the more explicitNumber.parseInt()method, which is the preferred approach in modern JavaScript.For even better clarity and to prevent potential issues with different string formats, consider specifying the radix:
Number.parseInt(d.next_l1_sequence, 10)This ensures the string is always parsed as a base-10 number.
src/core/ophost/BridgeConfig.ts (3)
142-142: LGTM: Replaced Buffer with polyfill function.The change from
Buffer.from(data.metadata).toString('base64')tobase64FromBytes(data.metadata)successfully replaces the Buffer usage with the new polyfill function, aligning with the PR objective.For improved clarity, consider adding a comment explaining the purpose of this conversion:
metadata: base64FromBytes(data.metadata) // Convert metadata from Uint8Array to base64 string
166-166: LGTM: Replaced Buffer with polyfill function.The change from
Buffer.from(metadata, 'base64')tobytesFromBase64(metadata)successfully replaces the Buffer usage with the new polyfill function, aligning with the PR objective. The null-check logic is correctly maintained.For consistency with the
fromProtomethod, consider adding a comment explaining the purpose of this conversion:metadata: metadata ? bytesFromBase64(metadata) : undefined, // Convert metadata from base64 string to Uint8Array if it exists
Line range hint
1-201: Summary: Successfully replaced Buffer with polyfill functions.The changes in this file consistently replace Buffer usage with polyfill functions (
base64FromBytesandbytesFromBase64). This aligns well with the PR objective of replacing Buffer with Uint8Array, improving compatibility across different JavaScript environments.The implementation is correct and consistent throughout the file, maintaining the existing logic while enhancing cross-platform compatibility. These changes should improve the portability of the code without introducing any functional changes.
Consider adding unit tests to verify that the serialization and deserialization processes using these new polyfill functions produce the same results as the previous Buffer-based implementation. This will ensure that the changes haven't introduced any subtle differences in behavior.
src/core/PublicKey.ts (2)
1-21: LGTM! Consider grouping related imports.The changes to import statements and constant definitions align well with the PR objective of replacing Buffer with Uint8Array. The use of
bytesFromHexfor constant definitions is consistent with this approach.Consider grouping related imports together for better readability. For example:
import { bech32 } from 'bech32' import { sha256, ripemd160 } from '../util/hash' import { JSONSerializable } from '../util/json' import { bytesFromBase64, bytesFromHex, base64FromBytes, concatBytes, } from '../util/polyfill' import { LegacyAminoPubKey as LegacyAminoPubKey_pb } from '@initia/initia.proto/cosmos/crypto/multisig/keys' import { PubKey as PubKey_pb } from '@initia/initia.proto/cosmos/crypto/secp256k1/keys' import { PubKey as ValConsPubKey_pb } from '@initia/initia.proto/cosmos/crypto/ed25519/keys' import { Any } from '@initia/initia.proto/google/protobuf/any'
Line range hint
1-377: Overall assessment: Excellent refactoring to replace Buffer with Uint8ArrayThe changes in this file consistently replace Buffer with Uint8Array-based utilities, aligning perfectly with the PR objective. Key improvements include:
- Consistent use of
bytesFromBase64,base64FromBytes, andconcatBytesutilities.- Updated method signatures with return types, enhancing type safety.
- Maintained functionality while modernizing the codebase.
These changes should improve compatibility and potentially performance, as stated in the PR objectives. The code is now more consistent and easier to maintain.
Consider adding unit tests to verify that the behavior of these methods remains unchanged after the refactoring, especially for edge cases involving different types of input data.
src/core/ibc/core/client/msgs/tendermint/types.ts (2)
2-5: Approve import changes and note potential impact.The addition of
base64FromBytesandbytesFromBase64imports aligns with the PR objective of replacing Buffer with Uint8Array. This change may have a widespread impact on the codebase, potentially improving cross-platform compatibility.Consider reviewing other parts of the codebase that may still be using Buffer for similar operations to ensure consistency across the project.
Line range hint
1-739: Summary of changes and their impact.This PR introduces two main changes to the
types.tsfile:
- Consistent renaming of properties from camelCase to snake_case across all classes.
- Replacement of Buffer with new utility functions (
base64FromBytesandbytesFromBase64) for base64 encoding/decoding.These changes align with the PR objective of replacing Buffer with Uint8Array and improve consistency in the codebase. The new naming convention may enhance readability for developers familiar with Python-style naming.
Key points to consider:
- The changes may impact other parts of the codebase that rely on these types.
- Performance implications of the new base64 handling should be verified.
- Thorough testing is crucial to ensure no functionality has been broken.
Consider updating the project's coding guidelines to reflect the new naming convention and base64 handling approach. This will ensure future contributions maintain consistency with these changes.
src/util/polyfill.ts (2)
1-1: Consider minimizing the use of ESLint disable directivesUsing extensive
eslint-disabledirectives reduces the effectiveness of linting. Instead of disabling multiple rules for the entire file, consider addressing the underlying issues or disabling rules for specific lines where necessary to maintain code quality.
28-107: Add unit tests for the newly introduced utility functionsComprehensive unit tests will ensure that the utility functions behave as expected across different environments and edge cases. This is especially important given the environment-specific code paths.
Would you like assistance in creating the unit tests for these functions?
src/core/wasm/msgs/MsgStoreAndInstantiateContract.ts (1)
2-2: Replace Node.js 'Buffer' with utility functions for broader compatibilityImporting
base64FromBytesandbytesFromBase64from../../../util/polyfillreplaces the use of Node.jsBuffer, enhancing compatibility with environments whereBufferis unavailable, such as browsers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (82)
- README.md (8 hunks)
- package.json (1 hunks)
- src/client/WebSocketClient.ts (4 hunks)
- src/client/lcd/LCDClient.ts (1 hunks)
- src/client/lcd/api/FeeGrantAPI.spec.ts (1 hunks)
- src/client/lcd/api/ForwardingAPI.ts (1 hunks)
- src/client/lcd/api/GovAPI.spec.ts (1 hunks)
- src/client/lcd/api/IbcAPI.spec.ts (2 hunks)
- src/client/lcd/api/IbcTransferAPI.spec.ts (1 hunks)
- src/client/lcd/api/OpchildAPI.ts (1 hunks)
- src/client/lcd/api/OphostAPI.ts (3 hunks)
- src/client/lcd/api/TendermintAPI.spec.ts (2 hunks)
- src/client/lcd/api/TxAPI.ts (2 hunks)
- src/core/PublicKey.ts (5 hunks)
- src/core/auction/AuctionParams.ts (3 hunks)
- src/core/auction/msgs/MsgAuctionBid.ts (3 hunks)
- src/core/bank/msgs/MsgMultiSend.ts (3 hunks)
- src/core/bech32.spec.ts (5 hunks)
- src/core/bech32.ts (3 hunks)
- src/core/celestia/Blob.ts (3 hunks)
- src/core/celestia/BlobTx.ts (1 hunks)
- src/core/celestia/msgs/MsgPayForBlobs.ts (3 hunks)
- src/core/ibc/core/channel/Packet.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgChannelCloseConfirm.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgChannelOpenAck.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgChannelOpenConfirm.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgChannelOpenTry.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgRecvAcknowledgement.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgRecvPacket.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgTimeout.ts (3 hunks)
- src/core/ibc/core/channel/msgs/MsgTimeoutClose.ts (3 hunks)
- src/core/ibc/core/client/Height.ts (1 hunks)
- src/core/ibc/core/client/msgs/MsgUpgradeClient.ts (3 hunks)
- src/core/ibc/core/client/msgs/tendermint/crypto.ts (5 hunks)
- src/core/ibc/core/client/msgs/tendermint/types.ts (19 hunks)
- src/core/ibc/core/commitment/MerklePrefix.ts (2 hunks)
- src/core/ibc/core/commitment/MerkleRoot.ts (2 hunks)
- src/core/ibc/core/connection/msgs/MsgConnectionOpenAck.ts (3 hunks)
- src/core/ibc/core/connection/msgs/MsgConnectionOpenConfirm.ts (3 hunks)
- src/core/ibc/core/connection/msgs/MsgConnectionOpenTry.ts (4 hunks)
- src/core/move/msgs/MsgExecute.ts (3 hunks)
- src/core/move/msgs/MsgGovExecute.ts (3 hunks)
- src/core/move/msgs/MsgGovPublish.ts (3 hunks)
- src/core/move/msgs/MsgGovScript.ts (3 hunks)
- src/core/move/msgs/MsgGovScriptJSON.ts (3 hunks)
- src/core/move/msgs/MsgPublish.ts (3 hunks)
- src/core/move/msgs/MsgScript.ts (2 hunks)
- src/core/move/msgs/MsgScriptJSON.ts (3 hunks)
- src/core/opchild/msgs/MsgFinalizeTokenDeposit.ts (3 hunks)
- src/core/opchild/msgs/MsgUpdateOracle.ts (3 hunks)
- src/core/ophost/BridgeConfig.ts (3 hunks)
- src/core/ophost/Output.ts (3 hunks)
- src/core/ophost/msgs/MsgFinalizeTokenWithdrawal.ts (3 hunks)
- src/core/ophost/msgs/MsgInitiateTokenDeposit.ts (3 hunks)
- src/core/ophost/msgs/MsgProposeOutput.ts (3 hunks)
- src/core/ophost/msgs/MsgRecordBatch.ts (3 hunks)
- src/core/ophost/msgs/MsgUpdateMetadata.ts (3 hunks)
- src/core/params/proposals/ParameterChangeProposal.ts (1 hunks)
- src/core/slashing/SlashingParams.ts (3 hunks)
- src/core/tx/CompactBitArray.ts (4 hunks)
- src/core/tx/SignDoc.ts (2 hunks)
- src/core/tx/SignatureV2.ts (2 hunks)
- src/core/tx/Tx.ts (3 hunks)
- src/core/wasm/authorizations/AcceptedMessagesFilter.ts (2 hunks)
- src/core/wasm/authorizations/CodeGrant.ts (3 hunks)
- src/core/wasm/msgs/MsgExecuteContract.ts (3 hunks)
- src/core/wasm/msgs/MsgInstantiateContract.ts (3 hunks)
- src/core/wasm/msgs/MsgInstantiateContractV2.ts (3 hunks)
- src/core/wasm/msgs/MsgMigrateContract.ts (3 hunks)
- src/core/wasm/msgs/MsgStoreAndInstantiateContract.ts (3 hunks)
- src/core/wasm/msgs/MsgStoreAndMigrateContract.ts (3 hunks)
- src/core/wasm/msgs/MsgStoreCode.ts (3 hunks)
- src/core/wasm/msgs/MsgSudoContract.ts (3 hunks)
- src/key/Key.ts (6 hunks)
- src/key/MnemonicKey.ts (3 hunks)
- src/key/RawKey.ts (2 hunks)
- src/key/ledger/app.ts (1 hunks)
- src/util/bcs.ts (2 hunks)
- src/util/hash.ts (0 hunks)
- src/util/index.ts (1 hunks)
- src/util/polyfill.spec.ts (1 hunks)
- src/util/polyfill.ts (1 hunks)
⛔ Files not processed due to max files limit (1)
- webpack.config.js
💤 Files with no reviewable changes (1)
- src/util/hash.ts
✅ Files skipped from review due to trivial changes (9)
- src/client/WebSocketClient.ts
- src/client/lcd/LCDClient.ts
- src/client/lcd/api/FeeGrantAPI.spec.ts
- src/client/lcd/api/GovAPI.spec.ts
- src/client/lcd/api/IbcAPI.spec.ts
- src/client/lcd/api/IbcTransferAPI.spec.ts
- src/core/bank/msgs/MsgMultiSend.ts
- src/core/params/proposals/ParameterChangeProposal.ts
- src/key/ledger/app.ts
🧰 Additional context used
🪛 Markdownlint
README.md
106-106: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
120-120: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
134-134: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
🔇 Additional comments (235)
src/util/index.ts (1)
4-4: LGTM! The new export enhances utility accessibility.The addition of
export * from './polyfill'is consistent with the existing export pattern and aligns with the PR objective of replacing the Buffer class with Uint8Array. This change makes the polyfill utilities accessible to other parts of the codebase that import from this index file.To ensure the change doesn't introduce any unintended side effects, please verify that all imports referencing this utility index are still valid. You can run the following script to check for any potential issues:
This script will help identify any potential conflicts or redundancies in imports across the codebase.
✅ Verification successful
Verification Successful: No Issues Detected
The shell scripts confirmed that all imports from the
utilmodule andutil/polyfillare intact and not broken by the new export. No unintended side effects were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for imports of the util module and verify they're not broken by the new export # Search for imports from the util module rg --type typescript --type javascript "from ['\"].*?/util['\"]" -g '!**/node_modules/**' # Check for any direct imports of polyfill that might now be redundant rg --type typescript --type javascript "from ['\"].*?/util/polyfill['\"]" -g '!**/node_modules/**'Length of output: 270
Script:
#!/bin/bash # Description: Check for imports of the util module and verify they're not broken by the new export # Search for imports from the util module rg --type ts --type js "from ['\"].*?/util['\"]" -g '!**/node_modules/**' # Check for any direct imports of polyfill that might now be redundant rg --type ts --type js "from ['\"].*?/util/polyfill['\"]" -g '!**/node_modules/**'Length of output: 6989
src/client/lcd/api/ForwardingAPI.ts (1)
Line range hint
1-45: Overall, the changes and file structure look good.The modifications to use
Number.parseIntare a minor improvement in code clarity. The file is well-structured, uses TypeScript features effectively, and maintains clear separation of concerns. The suggestions for adding error handling and specifying the radix parameter should further enhance the code's robustness and reliability.src/core/ibc/core/commitment/MerkleRoot.ts (4)
3-3: LGTM: New utility functions imported for base64 operations.The addition of
base64FromBytesandbytesFromBase64from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances cross-platform compatibility.
40-40: LGTM: Improved base64 encoding infromProtomethod.The use of
base64FromBytesinstead ofBuffer.from(...).toString('base64')enhances cross-platform compatibility and aligns with the PR objective. The functionality remains unchanged while simplifying the code.
45-45: LGTM: Improved base64 decoding intoProtomethod.The use of
bytesFromBase64instead ofBuffer.from(..., 'base64')enhances cross-platform compatibility and aligns with the PR objective. The functionality remains unchanged while simplifying the code.
Line range hint
1-58: Verify the impact of changes across the codebase.While the public interface of the
MerkleRootclass remains unchanged, the internal implementation offromProtoandtoProtomethods has been modified. To ensure the changes don't introduce any unintended side effects:
- Verify that the new utility functions (
base64FromBytesandbytesFromBase64) provide identical results to the previous Buffer-based implementation.- Update any tests that were expecting Buffer objects to work with the new Uint8Array-based implementation.
- Check for any code that might be directly accessing the
hashproperty and ensure it's compatible with the base64 string format.To help with verification, you can run the following script:
src/key/RawKey.ts (7)
5-5: LGTM: Import statement updated correctly.The new import for
base64FromBytesandbytesFromHexfrom '../util' is consistent with the changes in the class implementation, replacing Buffer operations with these utility functions.
15-15: LGTM: Property type updated to Uint8Array.The change from
BuffertoUint8Arrayfor theprivateKeyproperty is consistent with the PR objective and aligns with modern JavaScript practices. This update improves cross-platform compatibility.
17-21: LGTM: Constructor updated to use Uint8Array.The constructor has been correctly updated to accept a
Uint8Arrayinstead of aBuffer. ThepublicKeycreation andsupercall remain unchanged, maintaining the existing functionality. TheprivateKeyassignment is now direct, eliminating the need for type conversion.
26-26: LGTM: fromHex method updated to use bytesFromHex.The
fromHexmethod has been correctly updated to usebytesFromHexinstead ofBuffer.from. This change is consistent with the transition from Buffer to Uint8Array and maintains the existing hex string validation.
30-33: LGTM: sign method updated to use Uint8Array.The
signmethod has been correctly updated to accept and returnUint8Arrayinstead ofBuffer. The hash creation now usesbytesFromHex, which is consistent with the move away from Buffer. The signature is returned directly as aUint8Array, eliminating unnecessary type conversion.
37-43: LGTM: signWithKeccak256 method updated to use Uint8Array.The
signWithKeccak256method has been correctly updated to accept and returnUint8Arrayinstead ofBuffer. The keccak256 hash is now used directly without conversion to Buffer, and the signature is returned as aUint8Array, eliminating unnecessary type conversions.
Line range hint
1-45: Summary: Successful transition from Buffer to Uint8ArrayThe changes in this file successfully replace all instances of
BufferwithUint8Array, achieving the PR objective. This transition improves cross-platform compatibility and aligns the code with modern JavaScript practices. The modifications are consistent across the entire class, including property types, method signatures, and internal implementations. No issues or potential bugs were identified during the review.Key improvements:
- Enhanced compatibility by using
Uint8Arrayinstead of Node.js-specificBuffer.- Consistent use of utility functions (
bytesFromHex,base64FromBytes) for type conversions.- Simplified code by removing unnecessary type conversions.
These changes contribute to a more robust and portable implementation of the
RawKeyclass.src/core/ibc/core/commitment/MerklePrefix.ts (4)
3-3: LGTM: New import statement for polyfill functions.The addition of this import statement aligns with the PR objective of replacing Buffer operations with polyfill functions for base64 encoding and decoding.
49-49: LGTM: Replaced Buffer with polyfill function in fromProto method.The change successfully replaces the Buffer operation with the
base64FromBytespolyfill function, maintaining the same functionality of convertingkeyPrefixfrom bytes to a base64 string.
55-55: LGTM: Replaced Buffer with polyfill function in toProto method.The change successfully replaces the Buffer operation with the
bytesFromBase64polyfill function, maintaining the same functionality of convertingkey_prefixfrom a base64 string to bytes.
Line range hint
1-70: Overall: Excellent implementation of Buffer replacement.The changes in this file consistently replace Buffer operations with polyfill functions for base64 encoding and decoding. This aligns perfectly with the PR objective of replacing Buffer with Uint8Array for improved compatibility and performance. The functionality of the MerklePrefix class remains unchanged, and the implementation is consistent throughout the file.
src/client/lcd/api/TendermintAPI.spec.ts (2)
4-4: LGTM: New import statement forbytesFromBase64The import statement for
bytesFromBase64from the polyfill utility is correctly added. This aligns with the PR objective of replacing Buffer with Uint8Array for better compatibility.
Line range hint
11-19: Verify test results after Buffer replacementThe changes to replace
Buffer.fromwithbytesFromBase64are minimal and focused, which is good. However, it's crucial to ensure that this modification doesn't alter the behavior of the "load block and decode txs" test case.Please run the test suite and confirm that all tests, especially this modified one, still pass successfully. You can use the following command to run the tests:
If any issues are found, please investigate and address them before merging this PR.
src/core/celestia/Blob.ts (4)
2-2: LGTM: New utility functions imported for base64 operations.The addition of
base64FromBytesandbytesFromBase64from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances cross-platform compatibility.
51-52: LGTM: Buffer replaced with base64FromBytes in fromProto method.The use of
base64FromBytesinstead of Buffer operations improves cross-platform compatibility and aligns with the PR objectives. The functionality remains the same, but the code is now more consistent with the polyfill approach.
61-62: LGTM: Buffer replaced with bytesFromBase64 in toProto method.The use of
bytesFromBase64instead of Buffer operations improves cross-platform compatibility and aligns with the PR objectives. This change maintains symmetry with thefromProtomethod, ensuring consistent encoding and decoding operations throughout the class.
Line range hint
1-85: Overall assessment: Excellent refactoring to improve compatibility.The changes in this file successfully replace Buffer operations with polyfill utility functions (
base64FromBytesandbytesFromBase64). This refactoring enhances cross-platform compatibility without altering theBlobclass's functionality. The code remains clear and maintainable, with consistent use of the new utility functions in bothfromProtoandtoProtomethods.These modifications align perfectly with the PR objective of replacing the Buffer class with Uint8Array-based operations. Great job on improving the codebase's portability!
src/key/MnemonicKey.ts (3)
53-56: LGTM: Improved code example consistencyThe removal of semicolons from the code example improves consistency with the rest of the codebase and enhances readability.
62-63: LGTM: Consistent code example formattingThe removal of semicolons in this code example maintains the consistency established in the previous example.
77-77: Approve Uint8Array usage, but clarify sync method choiceThe change to wrap
privateKeyinUint8Array.from()is a good improvement. It ensures type consistency and aligns with the PR objective of replacing Buffer with Uint8Array.However, I noticed that
bip39.mnemonicToSeedSyncis now used instead of an async version. While this makes the constructor synchronous and potentially improves performance, it could block the main thread for large mnemonics.Could you please clarify the rationale behind switching to the sync version? Are there any performance benchmarks or specific use cases that prompted this change?
To verify the impact of this change, we can run the following script to check for any async usage of
MnemonicKeyin the codebase:This will help us understand if the switch to a synchronous constructor might cause issues in existing async contexts.
Also applies to: 86-86
src/client/lcd/api/OpchildAPI.ts (2)
46-46: Approve the use ofNumber.parseIntThe change from
parseInttoNumber.parseIntis a good improvement. It makes the code more explicit and robust, especially in environments where the globalparseIntfunction might have been modified. This change maintains the same functionality while slightly enhancing code quality.
54-54: Approve consistent use ofNumber.parseIntThis change mirrors the improvement made in the
nextL1Sequencemethod. UsingNumber.parseInthere maintains consistency throughout the class and brings the same benefits of explicitness and robustness.src/core/wasm/authorizations/AcceptedMessagesFilter.ts (2)
2-2: LGTM: New import for base64 utility functions.The addition of
base64FromBytesandbytesFromBytesimports from the polyfill utility aligns with the PR objective of replacing Buffer usage. This change promotes better compatibility and consistency across different environments.
44-44: LGTM: Replaced Buffer usage with base64FromBytes utility.The change effectively replaces the Buffer-based base64 decoding with the new
base64FromBytesutility function. This modification aligns with the PR objective of removing Buffer dependencies, potentially improving cross-platform compatibility and performance.src/core/wasm/authorizations/CodeGrant.ts (4)
2-2: LGTM: Import statement for polyfill functions.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill module aligns with the PR objective of replacing Buffer with Uint8Array. This change supports the modifications in thefromProtoandtoProtomethods.
56-56: LGTM: Replaced Buffer usage with polyfill function infromProto.The change from
Buffer.from(data.codeHash).toString('base64')tobase64FromBytes(data.codeHash)successfully replaces the Buffer usage with the imported polyfill function. This modification maintains the same functionality while aligning with the PR objective.
66-66: LGTM: Replaced Buffer usage with polyfill function intoProto.The change from
Buffer.from(code_hash, 'base64')tobytesFromBase64(code_hash)successfully replaces the Buffer usage with the imported polyfill function. This modification maintains the same functionality while aligning with the PR objective.
2-2: Summary: Successful replacement of Buffer with polyfill functions.The changes in this file consistently replace Buffer usage with the newly imported polyfill functions
base64FromBytesandbytesFromBase64. These modifications:
- Align with the PR objective of replacing Buffer with Uint8Array.
- Maintain the original functionality of base64 encoding and decoding.
- Improve cross-platform compatibility by using polyfills instead of Node.js-specific Buffer class.
The implementation is consistent throughout the file and achieves the intended goal without introducing any apparent issues.
Also applies to: 56-56, 66-66
package.json (4)
Line range hint
3-3: Version update looks good.The package version has been updated to "0.2.17". This change follows semantic versioning principles, which is a good practice for managing package releases.
59-59: Repositioning ofvite-plugin-dtsis fine.The
vite-plugin-dtsdependency has been moved to the end of the devDependencies list. This change doesn't affect the functionality and is likely due to automatic sorting or a preference for organizing dependencies.
Line range hint
1-91: Summary of package.json changesThe changes to
package.jsonalign with the PR objectives and seem to modernize the build process:
- The package version has been updated to "0.2.17".
- Several webpack-related devDependencies have been removed, suggesting a shift towards using Vite for building.
- The
vite-plugin-dtsdependency has been repositioned within the devDependencies list.These changes appear to be part of a larger effort to update the project's build process and potentially improve performance. The removal of the Buffer-related dependencies aligns with the stated goal of replacing Buffer with Uint8Array.
To ensure a smooth transition:
- Verify that the build process works correctly with the new configuration.
- Update any development workflows or CI/CD pipelines that might have depended on the removed webpack-related tools.
- Test the package thoroughly to ensure that the removal of these dependencies hasn't introduced any unexpected issues.
59-59: Verify build process after removing webpack-related dependencies.The removal of webpack-related dependencies (ts-loader, tsconfig-paths-webpack-plugin, webpack, webpack-bundle-analyzer, and webpack-cli) suggests a shift in the build process, possibly towards using Vite. This aligns with the PR objective of replacing Buffer with Uint8Array and modernizing the codebase.
Please ensure that:
- The build process still works as expected without these dependencies.
- All necessary build steps are now handled by Vite or other remaining tools.
- The development workflow and any CI/CD pipelines are updated to reflect these changes.
To verify the build process, you can run:
src/core/wasm/msgs/MsgSudoContract.ts (3)
2-2: LGTM: New import for base64 utility functions.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility module is consistent with the PR objective of replacing Buffer with Uint8Array. This change promotes better cross-platform compatibility.
63-63: LGTM: Replaced Buffer usage with polyfill function.The change from
Buffer.from(data.msg).toString('base64')tobase64FromBytes(data.msg)successfully eliminates the use of the Node.js-specific Buffer class. This modification enhances cross-platform compatibility while maintaining the same functionality of converting the message data to a base64 string.
72-72: LGTM: Replaced Buffer usage with polyfill function.The change from
Buffer.from(msg, 'base64')tobytesFromBase64(msg)successfully eliminates the use of the Node.js-specific Buffer class. This modification enhances cross-platform compatibility while maintaining the same functionality of converting the base64 string back to bytes.src/core/opchild/msgs/MsgUpdateOracle.ts (4)
2-2: LGTM: New import aligns with PR objectivesThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility module aligns well with the PR objective of replacing Buffer with Uint8Array. This change supports the goal of enhancing compatibility and performance within the codebase.
68-68: LGTM: Simplified base64 encodingThe use of
base64FromBytes(data.data)simplifies the code by directly converting the input to base64 without explicitly using Buffer. This change enhances readability and aligns with the PR's goal of replacing Buffer usage.
77-77: LGTM: Simplified base64 decodingThe use of
bytesFromBase64(data)simplifies the code by directly converting the base64 input to bytes without explicitly using Buffer. This change improves code clarity and aligns with the PR's objective of replacing Buffer usage.
Line range hint
1-114: Overall: Successful replacement of Buffer with polyfill utilitiesThe changes in this file successfully replace Buffer usage with the new polyfill utility functions for base64 encoding and decoding. This aligns perfectly with the PR objectives of enhancing compatibility and potentially improving performance. The modifications maintain the overall structure and functionality of the
MsgUpdateOracleclass while simplifying the code.To fully validate the impact of these changes:
- Ensure that the new polyfill functions (
base64FromBytesandbytesFromBase64) are thoroughly tested for correctness and edge cases.- Consider running performance benchmarks to verify any potential performance improvements, especially for large data sets.
To verify the implementation of the polyfill functions, you can run the following script:
src/core/ibc/core/channel/msgs/MsgRecvPacket.ts (4)
2-2: LGTM: Import polyfill functions for base64 encoding/decodingThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change provides the necessary functionality for base64 encoding and decoding without relying on the Node.js Buffer class.
64-64: LGTM: Update base64 encoding in fromProto methodThe change from Buffer to
base64FromBytesfor encodingproofCommitmentis correct and consistent with the PR objective. This modification maintains the method's functionality while removing the dependency on the Node.js Buffer class.
74-74: LGTM: Update base64 decoding in toProto methodThe change from Buffer to
bytesFromBase64for decodingproof_commitmentis correct and consistent with the PR objective. This modification maintains the method's functionality while removing the dependency on the Node.js Buffer class.
Line range hint
1-105: Overall assessment: Changes successfully implement Buffer replacementThe modifications in this file successfully replace the usage of the Node.js Buffer class with Uint8Array-based polyfill functions for base64 encoding and decoding. The changes are consistent throughout the file and maintain the functionality and backwards compatibility of the
MsgRecvPacketclass. These updates align well with the PR objective of enhancing compatibility and performance.src/core/ophost/msgs/MsgRecordBatch.ts (2)
2-2: Excellent addition of polyfill utilitiesThe import of
base64FromBytesandbytesFromBase64from the polyfill utility module aligns well with the PR objective of replacing theBufferclass. This change promotes better code organization and enhances compatibility across different environments.
Line range hint
1-114: Overall approval: Successful implementation of Buffer replacementThe changes in this file successfully replace the usage of the
Bufferclass with polyfill functions, aligning perfectly with the PR objective. Key points:
- Consistent use of
base64FromBytesandbytesFromBase64functions.- Localized changes to
fromProtoandtoProtomethods, minimizing potential side effects.- Improved cross-platform compatibility by reducing dependency on Node.js-specific APIs.
- Maintained functionality while simplifying the code.
These modifications contribute to a more portable and maintainable codebase. Great job on implementing these changes consistently and effectively!
src/core/auction/msgs/MsgAuctionBid.ts (4)
2-2: LGTM: Import of polyfill functions for base64 operationsThis import aligns with the PR objective of replacing Buffer with Uint8Array. The
base64FromBytesandbytesFromBase64functions are likely polyfills that provide consistent base64 encoding and decoding operations across different environments.
64-64: LGTM: Simplified base64 encoding infromProtomethodThis change replaces the previous Buffer-based implementation with the new
base64FromBytesfunction. It simplifies the code and makes it more consistent with the PR objective of replacing Buffer usage. The use ofmapensures that all transactions are properly encoded.
73-73: LGTM: Simplified base64 decoding intoProtomethodThis change replaces the previous Buffer-based implementation with the new
bytesFromBase64function. It simplifies the code and aligns with the PR objective of replacing Buffer usage. The use ofmapensures that all transactions are properly decoded when converting to the Proto format.
Line range hint
1-114: Summary: Successful replacement of Buffer with polyfill functionsThe changes in this file successfully replace the use of Buffer with new polyfill functions for base64 encoding and decoding. These modifications:
- Introduce new imports for
base64FromBytesandbytesFromBase64.- Update the
fromProtoandtoProtomethods to use these new functions.These changes align well with the PR objective of replacing the Buffer class with Uint8Array, enhancing compatibility and potentially improving performance. The implementation is consistent throughout the file and maintains the existing functionality while simplifying the code.
src/core/ophost/msgs/MsgUpdateMetadata.ts (4)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility module aligns with the PR objective of replacing Buffer with Uint8Array. These functions will provide the necessary base64 encoding and decoding operations without relying on the Node.js Buffer class, enhancing cross-platform compatibility.
73-73: LGTM: Replaced Buffer usage with new utility functionThe change from
Buffer.from(data.metadata).toString('base64')tobase64FromBytes(data.metadata)successfully eliminates the use of the Buffer class, aligning with the PR objective. This modification enhances cross-platform compatibility and maintains the same functionality using the new utility function. The clear function name also improves code readability.
82-82: LGTM: Consistent replacement of Buffer usageThe change from
Buffer.from(metadata, 'base64')tobytesFromBase64(metadata)is consistent with the previous modification in thefromProtomethod. It successfully removes the Buffer usage, replacing it with the newly imported utility function. This change maintains the desired functionality while improving cross-platform compatibility and code consistency throughout the class.
Line range hint
1-124: Summary: Successful replacement of Buffer with cross-platform utility functionsThe changes in this file consistently replace the usage of the Node.js Buffer class with newly imported utility functions (
base64FromBytesandbytesFromBase64). These modifications align perfectly with the PR objective of replacing Buffer with Uint8Array-based solutions. The implementation:
- Improves cross-platform compatibility
- Maintains the same functionality for base64 encoding and decoding
- Enhances code readability with clear function names
Overall, these changes contribute positively to the codebase by removing platform-specific dependencies and promoting a more universal approach to handling base64 operations.
src/core/ophost/Output.ts (4)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
72-72: LGTM: Replaced Buffer with base64FromBytes utilityThe change from Buffer-based conversion to
base64FromBytes(data.outputRoot)aligns with the PR objective of replacing Buffer usage. This modification maintains the same functionality while potentially improving performance and compatibility.
83-83: LGTM: Replaced Buffer with bytesFromBase64 utilityThe change from Buffer-based conversion to
bytesFromBase64(output_root)is in line with the PR objective of replacing Buffer usage. This modification maintains the same functionality of converting the base64 string back to bytes while potentially improving performance and compatibility.
Line range hint
1-108: Summary: Successfully replaced Buffer with polyfill utilitiesThe changes in this file consistently replace Buffer usage with the newly imported
base64FromBytesandbytesFromBase64utility functions. These modifications:
- Align with the PR objective of replacing the Buffer class with Uint8Array.
- Maintain the existing functionality for base64 encoding and decoding.
- Potentially improve performance and compatibility.
The implementation is consistent and well-executed across the file.
src/core/move/msgs/MsgPublish.ts (4)
2-2: LGTM: Import statement for polyfill functions added.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer usage. This change enhances compatibility across different JavaScript environments.
55-55: LGTM: Replaced Buffer usage with polyfill function infromProto.The modification to use
base64FromBytesinstead ofBuffer.from(code).toString('base64')simplifies the code and removes the dependency on the Node.js-specific Buffer class. This change improves cross-platform compatibility while maintaining the same functionality of converting byte arrays to base64 strings.
64-64: LGTM: Replaced Buffer usage with polyfill function intoProto.The modification to use
bytesFromBase64instead ofBuffer.from(code, 'base64')simplifies the code and removes the dependency on the Node.js-specific Buffer class. This change improves cross-platform compatibility while maintaining the same functionality of converting base64 strings to byte arrays.
2-2: Summary: Successful replacement of Buffer with polyfill functions.The changes in this file consistently replace Buffer usage with polyfill functions (
base64FromBytesandbytesFromBase64). These modifications achieve the following:
- Improve cross-platform compatibility by removing Node.js-specific Buffer dependencies.
- Maintain the same functionality for base64 encoding and decoding.
- Simplify the code by using direct polyfill function calls.
These changes align well with the PR objectives and contribute to making the codebase more portable across different JavaScript environments.
Also applies to: 55-55, 64-64
src/core/move/msgs/MsgScript.ts (4)
2-2: LGTM: New import statement for base64 utility functions.The import statement for
base64FromBytesandbytesFromBase64from the polyfill utility is correctly added. This import supports the replacement ofBufferusage with these utility functions throughout the file.
56-58: LGTM: Replaced Buffer usage with base64FromBytes utility function.The changes in the
fromProtomethod successfully replaceBufferusage with thebase64FromBytesutility function. This modification aligns with the PR objective of replacing Buffer with Uint8Array and makes the code more concise while maintaining the same functionality.
66-68: LGTM: Replaced Buffer usage with bytesFromBase64 utility function.The changes in the
toProtomethod successfully replaceBufferusage with thebytesFromBase64utility function. This modification aligns with the PR objective of replacing Buffer with Uint8Array and makes the code more concise while maintaining the same functionality.
Line range hint
1-134: Summary: Successfully replaced Buffer usage with base64 utility functions.The changes in this file consistently replace
Bufferusage with the new utility functionsbase64FromBytesandbytesFromBase64. These modifications align with the PR objective of replacing Buffer with Uint8Array and improve code consistency. The changes maintain the existing functionality while making the code more concise and potentially more performant.Key improvements:
- Added import for new utility functions.
- Updated
fromProtomethod to usebase64FromBytes.- Updated
toProtomethod to usebytesFromBase64.These changes contribute to better compatibility and performance within the codebase.
src/core/move/msgs/MsgScriptJSON.ts (4)
2-2: LGTM: Import statement for base64 utility functions.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer usage. This change enhances compatibility and maintains consistent coding style.
72-72: LGTM: Improved base64 encoding infromProtomethod.The replacement of
Buffer.from(data.codeBytes).toString('base64')withbase64FromBytes(data.codeBytes)successfully eliminates the Buffer usage, aligning with the PR objective. This change simplifies the code and potentially improves performance while maintaining the same functionality.
82-82: LGTM: Improved base64 decoding intoProtomethod.The replacement of
Buffer.from(code_bytes, 'base64')withbytesFromBase64(code_bytes)successfully eliminates the Buffer usage, aligning with the PR objective. This change simplifies the code and potentially improves performance while maintaining the same functionality.
Line range hint
1-124: Overall assessment: Changes successfully implement Buffer replacement.The modifications in this file consistently replace Buffer usage with polyfill functions for base64 encoding and decoding. These changes align perfectly with the PR objective of enhancing compatibility and performance by replacing the Buffer class with Uint8Array. The existing functionality is maintained, and the code becomes more concise. No further changes are necessary for this file within the current scope of the PR.
src/core/tx/SignDoc.ts (1)
2-2: LGTM: New import aligns with PR objectivesThe addition of the
base64FromBytesimport from the polyfill utility is consistent with the PR's goal of replacing Buffer with Uint8Array. This change supports the transition to a more compatible implementation.src/core/wasm/msgs/MsgExecuteContract.ts (2)
2-2: LGTM: Appropriate utility functions importedThe addition of
base64FromBytesandbytesFromBase64from the polyfill utility aligns well with the PR objective of replacing the Buffer class. This change should improve cross-platform compatibility and consistency in base64 encoding/decoding operations.
Line range hint
1-118: Overall: Good implementation of Buffer replacementThe changes in this file successfully replace Buffer usage with polyfill utility functions, improving cross-platform compatibility without altering the class's behavior. This aligns well with the PR objective.
To ensure the changes haven't introduced any regressions, please update the relevant unit tests to cover these modifications. Run the following command to check for existing tests:
If tests exist, update them to explicitly check the encoding/decoding with the new polyfill functions. If no tests are found, consider adding new ones to cover this functionality.
src/core/ibc/core/channel/msgs/MsgChannelOpenConfirm.ts (3)
2-2: LGTM: Import of base64 utility functionsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
73-73: LGTM: Updated base64 encoding in fromProto methodThe replacement of Buffer usage with
base64FromBytes(proto.proofAck)is correct and aligns with the PR objective. This change maintains the existing functionality while improving compatibility and potentially enhancing performance.
84-84: LGTM: Updated base64 decoding in toProto methodThe replacement of Buffer usage with
bytesFromBase64(proof_ack)is correct and aligns with the PR objective. This change maintains the existing functionality while improving compatibility and potentially enhancing performance.src/core/ibc/core/connection/msgs/MsgConnectionOpenConfirm.ts (4)
2-2: LGTM: Import of polyfill utilitiesThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns well with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
69-69: LGTM: Replacement of Buffer with polyfill utility in fromProtoThe change from
Buffer.from(proto.proofAck).toString('base64')tobase64FromBytes(proto.proofAck)successfully replaces the Buffer usage with the new polyfill utility. This modification maintains the same functionality while aligning with the PR's objective.
79-79: LGTM: Replacement of Buffer with polyfill utility in toProtoThe change from
Buffer.from(proof_ack, 'base64')tobytesFromBase64(proof_ack)successfully replaces the Buffer usage with the new polyfill utility. This modification maintains the same functionality while aligning with the PR's objective.
Line range hint
1-108: Summary: Successful replacement of Buffer with polyfill utilitiesThe changes in this file consistently replace Buffer usage with new polyfill utilities (
base64FromBytesandbytesFromBase64). These modifications:
- Align with the PR objective of replacing Buffer with Uint8Array.
- Maintain the original functionality of base64 encoding and decoding.
- Potentially enhance compatibility and performance.
The changes are well-implemented and consistent throughout the file. No issues or potential improvements were identified.
src/core/wasm/msgs/MsgMigrateContract.ts (2)
2-2: LGTM: Import of base64 utility functions aligns with PR objectives.The addition of
base64FromBytesandbytesFromBase64from the polyfill utility is consistent with the PR's goal of replacing the Buffer class. This change should enhance compatibility and potentially improve performance.
Line range hint
1-124: Consider updating related tests and documentation.The changes to
fromProtoandtoProtomethods are well-implemented and consistent with the PR objectives. To ensure completeness:
- Update any unit tests related to these methods to reflect the new implementation.
- Review and update any documentation that might reference the old Buffer-based implementation.
This will help maintain the overall quality and consistency of the codebase.
To assist in identifying related tests and documentation, you can run the following script:
src/core/ibc/core/channel/msgs/MsgChannelCloseConfirm.ts (4)
2-2: LGTM: Import of base64 utility functionsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances cross-platform compatibility.
73-73: LGTM: Updated base64 encoding infromProtoThe replacement of
Buffer.from(proto.proofInit).toString('base64')withbase64FromBytes(proto.proofInit)is consistent with the PR objective. This change maintains the same functionality while improving cross-platform compatibility.
84-84: LGTM: Updated base64 decoding intoProtoThe replacement of
Buffer.from(proof_init, 'base64')withbytesFromBase64(proof_init)is consistent with the PR objective. This change maintains the same functionality while improving cross-platform compatibility.
Line range hint
1-114: Summary: Successful replacement of Buffer with cross-platform base64 utilitiesThe changes in this file consistently replace Buffer usage with new utility functions (
base64FromBytesandbytesFromBase64) for base64 encoding and decoding. These modifications align perfectly with the PR objective of enhancing cross-platform compatibility.Key points:
- Import statements have been updated to include the new utility functions.
- The
fromProtoandtoProtomethods now use these new functions instead of Buffer.- The core functionality of the
MsgChannelCloseConfirmclass remains unchanged.These changes should improve the codebase's compatibility across different JavaScript environments while maintaining the existing behavior.
src/core/ibc/core/channel/msgs/MsgRecvAcknowledgement.ts (3)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances cross-platform compatibility and performance.
68-69: LGTM: Replaced Buffer usage with base64FromBytes utilityThe changes in the
fromProtomethod effectively replace the use ofBufferwith thebase64FromBytesutility function. This modification aligns with the PR objective and enhances cross-platform compatibility while maintaining the method's functionality.
Line range hint
1-114: Summary: Successful implementation of Buffer replacement with Uint8ArrayThe changes in this file successfully replace the use of
BufferwithUint8Array-based utility functions for base64 encoding and decoding operations. This modification enhances cross-platform compatibility and aligns with the PR objectives. The implementation maintains the existing functionality while improving the code's portability.Key improvements:
- Added imports for
base64FromBytesandbytesFromBase64utility functions.- Updated
fromProtoandtoProtomethods to use these new utility functions.These changes contribute to a more consistent and platform-independent codebase.
src/core/ibc/core/channel/msgs/MsgTimeout.ts (4)
2-2: LGTM: Import of polyfill utilitiesThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change sets the foundation for the subsequent modifications in the file.
81-81: LGTM: Utilization ofbase64FromBytesinfromProtoThe replacement of direct assignment with
base64FromBytes(proto.proofUnreceived)effectively implements the PR's objective. This change ensures that theproof_unreceivedis properly converted from bytes to a base64 string, maintaining the method's functionality while moving away from Buffer operations.
98-98: LGTM: Utilization ofbytesFromBase64intoProtoThe replacement of
Buffer.from(proof_unreceived, 'base64')withbytesFromBase64(proof_unreceived)successfully implements the PR's objective. This change ensures that theproof_unreceivedis correctly converted from a base64 string to bytes, maintaining the method's functionality while eliminating the use of Buffer operations.
Line range hint
1-124: Summary: Successful implementation of Buffer replacementThe changes in this file effectively implement the PR objective of replacing Buffer operations with Uint8Array-based alternatives. The modifications include:
- Importing necessary polyfill utilities.
- Updating the
fromProtomethod to usebase64FromBytes.- Updating the
toProtomethod to usebytesFromBase64.These changes maintain the functionality of the
MsgTimeoutclass while enhancing compatibility and potentially improving performance. The implementation is clean, consistent, and aligns well with the project's goals.src/core/celestia/msgs/MsgPayForBlobs.ts (4)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill module aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
76-78: LGTM: Replaced Buffer usage with polyfill function in fromProto methodThe changes in the
fromProtomethod effectively replaceBuffer.from(...).toString('base64')withbase64FromBytes. This modification:
- Removes the dependency on the Node.js Buffer class.
- Simplifies the code.
- Maintains the same functionality of converting byte arrays to base64 strings.
These changes are in line with the PR objective and improve cross-platform compatibility.
93-95: LGTM: Replaced Buffer usage with polyfill function in toProto methodThe modifications in the
toProtomethod effectively replaceBuffer.from(..., 'base64')withbytesFromBase64. This change:
- Eliminates the dependency on the Node.js Buffer class.
- Simplifies the code.
- Preserves the same functionality of converting base64 strings to byte arrays.
These alterations align with the PR objective and enhance cross-platform compatibility.
Line range hint
1-124: Summary: Successful replacement of Buffer with polyfill functionsThe changes in this file consistently replace Buffer usage with polyfill functions (
base64FromBytesandbytesFromBase64) throughout theMsgPayForBlobsclass. These modifications:
- Achieve the PR objective of replacing Buffer with Uint8Array-based solutions.
- Enhance cross-platform compatibility by removing Node.js-specific Buffer dependencies.
- Potentially improve performance through the use of specialized polyfill functions.
- Maintain the core functionality of the
MsgPayForBlobsclass.The changes are well-implemented and align perfectly with the PR's goals. Great job on improving the codebase!
src/core/bech32.spec.ts (4)
8-8: LGTM: New import statement for polyfill function.The addition of
import { bytesFromUtf8 } from '../util/polyfill'is consistent with the PR objective of replacing Buffer with Uint8Array. This import provides the necessary function to convert UTF-8 strings to byte arrays without relying on the Node.js Buffer class.
45-45: LGTM: Consistent use of bytesFromUtf8.The change from
Buffer.from('foobar', 'utf8')tobytesFromUtf8('foobar')is consistent with the previous modification and correctly implements the PR objective.
Line range hint
1-118: Overall assessment: Changes successfully implement PR objective.The modifications in this file consistently replace
Buffer.from('foobar', 'utf8')withbytesFromUtf8('foobar')across all test cases. This change aligns with the PR objective of replacing the Buffer class with a polyfill implementation, likely using Uint8Array.Key points:
- The new import statement for
bytesFromUtf8is correctly added.- All instances of Buffer usage have been replaced with the new polyfill function.
- The overall structure and logic of the tests remain unchanged.
These changes should improve cross-platform compatibility while maintaining the same functionality. Consider the suggested improvements in the previous comments to further enhance the code quality.
70-70: LGTM: Consistent use of bytesFromUtf8. Verify polyfill implementation.The change from
Buffer.from('foobar', 'utf8')tobytesFromUtf8('foobar')is consistent with the previous modifications and correctly implements the PR objective.To ensure the polyfill function behaves identically to the Buffer implementation across all test cases, run the following script:
This script will help verify that the
bytesFromUtf8function produces the same output asBuffer.fromfor the test string 'foobar'.src/core/wasm/msgs/MsgStoreCode.ts (4)
2-2: Approve: Import of polyfill utilities enhances compatibilityThe addition of
base64FromBytesandbytesFromBytesimports from the polyfill utility is a positive change. This aligns with the PR objective of replacing Buffer with Uint8Array, which should improve cross-platform compatibility and reduce dependencies on Node.js-specific APIs.
75-75: Approve: Consistent use of polyfill utility for base64 encodingThe replacement of
Buffer.from(data.wasmByteCode).toString('base64')withbase64FromBytes(data.wasmByteCode)is a positive change. It's consistent with the PR objective of replacing Buffer usage and utilizes the newly imported polyfill utility. This change enhances code consistency and maintainability.
86-86: Approve: Consistent use of polyfill utility for base64 decodingThe replacement of
Buffer.from(wasm_byte_code, 'base64')withbytesFromBase64(wasm_byte_code)is a positive change. It aligns with the PR objective of replacing Buffer usage and utilizes the newly imported polyfill utility. This change enhances code consistency and maintainability across the codebase.
Line range hint
1-124: Overall: Successful replacement of Buffer with polyfill utilitiesThe changes in this file successfully replace the usage of Node.js Buffer with polyfill utilities for base64 encoding and decoding. This aligns perfectly with the PR objective and should improve cross-platform compatibility. The functionality of the
MsgStoreCodeclass remains unchanged, while the code becomes more consistent and potentially more readable.While the impact on performance is likely minimal, it might be worth verifying that there's no significant performance regression, especially if these operations are performed frequently.
To ensure there's no significant performance impact, you may want to run the following performance test:
src/core/move/msgs/MsgGovPublish.ts (4)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
77-77: LGTM: Improved base64 encoding infromProtomethodThe modification to use
base64FromByteswithmapis a good improvement. It aligns with the PR objective of replacing Buffer usage, potentially improving cross-platform compatibility and performance. The functionality remains unchanged, converting byte data to base64 strings for each element incodeBytes.
87-87: LGTM: Enhanced base64 decoding intoProtomethodThe update to use
bytesFromBase64withmapis a solid improvement. It successfully removes the Buffer usage, aligning with the PR's goal and potentially enhancing cross-platform compatibility and performance. The core functionality remains intact, converting base64 strings back to byte data for each element incode_bytes.
Line range hint
1-124: Overall: Excellent implementation of Buffer replacementThe changes in this file consistently and effectively implement the PR objective of replacing Buffer with Uint8Array. The modifications to the
fromProtoandtoProtomethods, along with the new utility function imports, improve code consistency and potentially enhance cross-platform compatibility and performance. The core functionality of theMsgGovPublishclass remains intact, and the changes are well-integrated into the existing codebase.src/core/move/msgs/MsgGovScript.ts (4)
2-2: LGTM: Import statement for base64 utility functions added.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility is consistent with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and aligns with the overall goal of the pull request.
83-85: LGTM: Replaced Buffer usage with base64 utility functions infromProto.The changes in the
fromProtomethod successfully replace the use ofBufferwith the newbase64FromBytesutility function. This modification is applied consistently to bothcode_bytesandargs, maintaining the method's functionality while improving compatibility and aligning with the PR's objectives.
94-96: LGTM: Replaced Buffer usage with base64 utility functions intoProto.The modifications in the
toProtomethod successfully replace the use ofBufferwith the newbytesFromBase64utility function. This change is applied consistently to bothcode_bytesandargs, maintaining the method's functionality while improving compatibility. The symmetry between these changes and those in thefromProtomethod ensures consistency in the serialization and deserialization process, aligning perfectly with the PR's objectives.
Line range hint
1-138: Overall assessment: Changes successfully implement the PR objectives.The modifications in this file consistently replace the use of
Bufferwith new base64 utility functions (base64FromBytesandbytesFromBase64). These changes are well-implemented across theMsgGovScriptclass, particularly in thefromProtoandtoProtomethods. The alterations maintain the class's functionality while improving compatibility, which aligns perfectly with the PR's goal of replacing the Buffer class with Uint8Array.The consistency and symmetry of the changes in both serialization and deserialization methods ensure that the class's behavior remains intact. No issues or potential improvements were identified during this review.
src/core/ibc/core/client/msgs/tendermint/crypto.ts (5)
36-41: LGTM: Consistent property naming in fromData method.The renaming of
leafHashtoleaf_hashin the destructuring assignment is consistent with the changes made in the constructor. The method implementation remains correct, with only the property name being updated.
46-51: LGTM: Consistent property naming in toData method.The renaming of
leafHashtoleaf_hashin both the destructuring assignment and the returned object is consistent with the previous changes. The method implementation remains correct, with only the property name being updated.
60-61: LGTM: Improved base64 conversion using new utility function.The use of
base64FromBytesinstead ofBuffer.from(...).toString('base64')aligns with the PR objective of replacing Buffer with Uint8Array. This change results in a more concise implementation and leverages the imported utility function.Let's verify that
base64FromBytesis used consistently for similar conversions:#!/bin/bash # Description: Verify consistent usage of base64FromBytes for conversions from bytes to base64. # Test: Search for any remaining usage of Buffer.from(...).toString('base64') rg --type typescript 'Buffer\.from\(.*\)\.toString\(.(base64|ascii|utf8|utf-8|utf16le|ucs2|ucs-2|binary|latin1).\)' src/ # Test: Search for usage of base64FromBytes rg --type typescript 'base64FromBytes' src/
66-71: LGTM: Improved base64 conversion and consistent property naming.The changes in this method are twofold and both positive:
- The use of
bytesFromBase64instead ofBuffer.from(..., 'base64')aligns with the PR objective of replacing Buffer with Uint8Array.- The renaming of
leafHashtoleaf_hashmaintains consistency with previous changes.These modifications result in a more consistent and modern implementation.
Let's verify that
bytesFromBase64is used consistently for similar conversions:#!/bin/bash # Description: Verify consistent usage of bytesFromBase64 for conversions from base64 to bytes. # Test: Search for any remaining usage of Buffer.from(..., 'base64') rg --type typescript 'Buffer\.from\(.*,\s*.(base64|ascii|utf8|utf-8|utf16le|ucs2|ucs-2|binary|latin1).\)' src/ # Test: Search for usage of bytesFromBase64 rg --type typescript 'bytesFromBase64' src/
2-5: 💡 Codebase verificationLGTM: New utility functions imported for base64 conversions.
The addition of
base64FromBytesandbytesFromBase64imports aligns with the PR objective of replacing Buffer with Uint8Array. These utility functions are used consistently throughout the file for base64 conversions.Let's verify the usage of these new imports:
✅ Verification successful
Please verify the usage of the new base64 conversion functions by running the updated shell script below:
LGTM: The new base64 conversion utilities are correctly imported and utilized throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new base64 conversion utility functions. # Test: Search for usage of base64FromBytes and bytesFromBase64 rg --type typescript 'base64FromBytes|bytesFromBase64' src/Length of output: 236
Script:
#!/bin/bash # Description: Verify the usage of new base64 conversion utility functions using specific file extensions. # Search for usage of base64FromBytes and bytesFromBase64 in .ts and .tsx files rg 'base64FromBytes|bytesFromBase64' --glob '**/*.ts' --glob '**/*.tsx' src/Length of output: 24257
src/core/move/msgs/MsgGovScriptJSON.ts (4)
2-2: LGTM: Import of base64 utility functionsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
83-83: LGTM: Replacement of Buffer with base64FromBytesThe change from
Buffer.from(data.codeBytes).toString('base64')tobase64FromBytes(data.codeBytes)successfully replaces the Buffer usage with the new utility function. This modification aligns with the PR objective and likely improves the efficiency of the base64 encoding process.
94-94: LGTM: Replacement of Buffer with bytesFromBase64The change from
Buffer.from(code_bytes, 'base64')tobytesFromBase64(code_bytes)successfully replaces the Buffer usage with the new utility function. This modification aligns with the PR objective and likely improves the efficiency of the base64 decoding process.
Line range hint
1-138: Summary: Successful replacement of Buffer with base64 utility functionsThe changes in this file consistently replace Buffer usage with the newly imported
base64FromBytesandbytesFromBase64utility functions. These modifications align perfectly with the PR objective of replacing Buffer with Uint8Array for improved compatibility and performance.Key improvements:
- Import of specialized base64 utility functions.
- Replacement of Buffer in
fromProtomethod for encoding.- Replacement of Buffer in
toProtomethod for decoding.These changes should result in more efficient base64 encoding and decoding operations while maintaining the existing functionality. The implementation is clean and consistent throughout the file.
src/core/ophost/msgs/MsgInitiateTokenDeposit.ts (4)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances cross-platform compatibility.
93-93: LGTM: Replaced Buffer usage with polyfill utilityThe
fromProtomethod now usesbase64FromBytes(msgProto.data)instead ofBuffer.from(msgProto.data).toString('base64'). This change aligns with the PR objective of replacing Buffer usage and maintains the same functionality of converting binary data to a base64 string.
104-104: LGTM: Replaced Buffer usage with polyfill utilityThe
toProtomethod now usesbytesFromBase64(data)instead ofBuffer.from(data, 'base64'). This change aligns with the PR objective of replacing Buffer usage and maintains the same functionality of converting a base64 string to binary data.
Line range hint
1-150: Summary: Successfully replaced Buffer with polyfill utilitiesThe changes in this file successfully replace Buffer usage with polyfill utilities for base64 encoding and decoding. This improves cross-platform compatibility while maintaining the existing functionality. Great job on implementing these changes consistently throughout the file!
To ensure the polyfill implementations provide the same level of functionality and performance as the Buffer class, please run the following verification script:
This script will help verify the implementation of the polyfill utilities and ensure that Buffer is no longer used in this file.
✅ Verification successful
Verification Successful: Polyfill Utilities Implemented and Buffer Usage Eliminated
The polyfill functions
base64FromBytesandbytesFromBase64are correctly implemented insrc/util/polyfill.ts. Additionally, there is no usage ofBufferinsrc/core/ophost/msgs/MsgInitiateTokenDeposit.ts, and no related TODOs or FIXMEs remain.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of base64 polyfill utilities # Test 1: Check if the polyfill utilities are implemented echo "Checking polyfill implementation..." rg --type typescript "export function (base64FromBytes|bytesFromBase64)" src/util/polyfill.ts # Test 2: Verify that Buffer is not used in this file echo "Verifying Buffer is not used..." rg --type typescript "Buffer\." src/core/ophost/msgs/MsgInitiateTokenDeposit.ts # Test 3: Check for any remaining TODOs or FIXMEs related to this change echo "Checking for remaining TODOs or FIXMEs..." rg --type typescript "(TODO|FIXME).*Buffer" src/core/ophost/msgs/MsgInitiateTokenDeposit.tsLength of output: 623
Script:
#!/bin/bash # Description: Verify the implementation and usage of base64 polyfill utilities # Test 1: Check if the polyfill utilities are implemented echo "Checking polyfill implementation..." rg --glob '*.ts' "export function (base64FromBytes|bytesFromBase64)" src/util/polyfill.ts # Test 2: Verify that Buffer is not used in this file echo "Verifying Buffer is not used..." rg --glob '*.ts' "Buffer\." src/core/ophost/msgs/MsgInitiateTokenDeposit.ts # Test 3: Check for any remaining TODOs or FIXMEs related to this change echo "Checking for remaining TODOs or FIXMEs..." rg --glob '*.ts' "(TODO|FIXME).*Buffer" src/core/ophost/msgs/MsgInitiateTokenDeposit.tsLength of output: 624
src/util/polyfill.spec.ts (2)
1-41: LGTM: Imports and base64 tests are well-structured and comprehensive.The imports align with the PR objectives of replacing Buffer with Uint8Array. The base64 tests cover essential scenarios:
- Converting base64 string to Uint8Array
- Handling empty base64 strings
- Converting Uint8Array to base64 string
- Handling empty Uint8Array
The test data and expected results are correct, ensuring robust coverage of the base64 utility functions.
1-121: Overall, excellent test coverage and structure.This new test file provides comprehensive coverage for the polyfill utility functions, aligning well with the PR objectives of replacing Buffer with Uint8Array. The tests are well-structured, consistent across all three encoding types (base64, hex, and UTF-8), and cover both normal conversions and edge cases.
A few minor suggestions:
- Consider adding tests for error cases, such as invalid input strings.
- You might want to add a test case for a more complex UTF-8 string containing non-ASCII characters to ensure proper handling of multi-byte characters.
These additions would further strengthen the robustness of your test suite.
src/core/ibc/core/channel/msgs/MsgTimeoutClose.ts (4)
2-2: LGTM: Import of base64 utility functionsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer usage. This change enhances cross-platform compatibility.
87-88: LGTM: Replacement of Buffer with base64FromBytesThe use of
base64FromBytesinstead of Buffer forproofUnreceivedandproofCloseis consistent with the PR objective. This change enhances cross-platform compatibility while maintaining the method's functionality.
106-107: LGTM: Replacement of Buffer with bytesFromBase64The use of
bytesFromBase64instead of Buffer forproofUnreceivedandproofCloseis consistent with the PR objective and symmetrical to the changes infromProto. This change enhances cross-platform compatibility while maintaining the method's functionality.
Line range hint
1-150: Overall assessment: Changes successfully implement Buffer replacementThe modifications in this file consistently replace Buffer usage with new utility functions for base64 encoding and decoding. These changes align perfectly with the PR objective of replacing the Buffer class with Uint8Array, enhancing cross-platform compatibility without altering the
MsgTimeoutOnCloseclass's functionality. The implementation is clean and maintains the existing structure of the code.src/core/ibc/core/client/msgs/MsgUpgradeClient.ts (4)
2-2: LGTM: Import of base64 utility functions aligns with PR objectives.The addition of
base64FromBytesandbytesFromBase64from the polyfill utility is in line with the PR's goal of replacing Buffer with Uint8Array. This change should enhance compatibility and potentially improve performance.
87-88: LGTM: Improved clarity in proof data conversion.The use of
base64FromBytesfor convertingproofUpgradeClientandproofUpgradeConsensusStateenhances code clarity and aligns with the PR's objective of removing Buffer usage. This change ensures consistent handling of byte array to base64 string conversion.
106-108: LGTM: Consistent approach to base64 decoding.The use of
bytesFromBase64for convertingproof_upgrade_clientandproof_upgrade_consensus_stateensures consistency with the PR's objective of removing Buffer usage. This change provides a clear and uniform approach to decoding base64 strings into byte arrays.The multi-line formatting for the longer property name also improves code readability.
Line range hint
1-141: Overall assessment: Changes successfully implement PR objectives.The modifications in this file consistently replace Buffer usage with the new utility functions
base64FromBytesandbytesFromBase64. These changes:
- Enhance compatibility by removing Node.js-specific Buffer usage.
- Potentially improve performance through the use of Uint8Array.
- Maintain the original functionality of the
MsgUpgradeClientclass.- Improve code clarity and consistency in handling base64 encoding and decoding.
The changes are well-implemented and align perfectly with the PR's objectives.
src/core/ibc/core/channel/msgs/MsgChannelOpenAck.ts (2)
95-95: LGTM! Verify thebase64FromBytesimplementation.The replacement of
Buffer.from(proto.proofTry).toString('base64')withbase64FromBytes(proto.proofTry)aligns with the PR objective and should improve compatibility and performance.To ensure the
base64FromBytesfunction is correctly implemented, run the following script:#!/bin/bash # Description: Verify the implementation of base64FromBytes function # Test: Check the implementation of base64FromBytes rg -t typescript "function base64FromBytes" -A 10 -g "src/**/util/polyfill.ts"
116-116: LGTM! Verify thebytesFromBase64implementation.The replacement of
Buffer.from(proof_try, 'base64')withbytesFromBase64(proof_try)aligns with the PR objective and should improve compatibility and performance.To ensure the
bytesFromBase64function is correctly implemented, run the following script:✅ Verification successful
Verified the
bytesFromBase64implementation.The
bytesFromBase64function insrc/util/polyfill.tscorrectly replacesBuffer.from(proof_try, 'base64'), ensuring compatibility and performance improvements as intended by the PR.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of bytesFromBase64 function # Test: Check the implementation of bytesFromBase64 rg -t typescript "function bytesFromBase64" -A 10 -g "src/**/util/polyfill.ts"Length of output: 115
Script:
#!/bin/bash # Description: Verify the implementation of bytesFromBase64 function # Search for the bytesFromBase64 function in .ts files rg "function bytesFromBase64" -A 10 -g "*.ts"Length of output: 598
src/core/ophost/msgs/MsgProposeOutput.ts (3)
2-2: LGTM: Import of polyfill functionsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill module aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility across different JavaScript environments.
107-107: LGTM: Use of bytesFromBase64 in toProto methodThe replacement of
Buffer.from(output_root, 'base64')withbytesFromBase64(output_root)is consistent with the PR objective and improves cross-environment compatibility. The null check is properly maintained, ensuring that undefined values are handled correctly.
Line range hint
1-150: Summary: Successful replacement of Buffer with polyfill functionsThe changes in this file successfully replace the usage of
Bufferwith polyfill functionsbase64FromBytesandbytesFromBase64. This aligns well with the PR objective of enhancing compatibility across different JavaScript environments.Key points:
- The new import statement adds the necessary polyfill functions.
- Both
fromProtoandtoProtomethods have been updated to use these new functions.- The overall functionality of the
MsgProposeOutputclass remains intact.These changes improve the code's portability without introducing any apparent issues or regressions. The minor suggestion for handling potential undefined values in the
fromProtomethod is the only point to consider for further improvement.src/core/ibc/core/channel/msgs/MsgChannelOpenTry.ts (1)
2-2: LGTM: Import of polyfill utilities.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.src/core/move/msgs/MsgExecute.ts (2)
2-2: Approved: New import aligns with PR objectivesThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility is consistent with the PR's goal of replacing Buffer with Uint8Array for improved compatibility.
Line range hint
1-186: Overall assessment: Changes are consistent and align with PR objectivesThe modifications in this file successfully replace Buffer usage with polyfill functions, enhancing compatibility as intended. The changes are consistent throughout the file and maintain the original functionality. Minor suggestions for improved clarity through comments have been provided.
src/core/wasm/msgs/MsgInstantiateContract.ts (3)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill module aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and potentially improves performance.
102-102: LGTM: Improved base64 decoding infromProtomethodThe use of
base64FromBytes(data.msg)simplifies the decoding process and aligns with the PR objective of replacing Buffer operations. This change enhances code readability and potentially improves performance.
Line range hint
1-164: Summary: Successful implementation of Buffer replacementThe changes in this file successfully replace Buffer operations with new polyfill functions for base64 encoding and decoding. These modifications align well with the PR objectives and improve code consistency. The implementation enhances compatibility and potentially improves performance.
Key points:
- New utility functions are imported from the polyfill module.
- Both
fromProtoandtoProtomethods now use these new functions for handling themsgproperty.- The changes simplify the code by removing buffer manipulations.
Overall, the modifications achieve the intended goal while maintaining the class's functionality.
src/core/ibc/core/channel/Packet.ts (4)
2-2: LGTM: Import changes look goodThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility is appropriate for the Buffer replacement changes. The reordering of thePacketimport is a minor change that doesn't affect functionality.Also applies to: 4-4
131-131: LGTM: Buffer replacement in fromProto methodThe change from
Buffer.from(proto.data).toString('base64')tobase64FromBytes(proto.data)aligns with the PR objective of replacing Buffer usage. This new implementation should provide the same functionality.To ensure the correctness of this change, please verify that
base64FromByteshandles the input correctly for all possible data types thatproto.datamight contain. Consider adding unit tests to cover various scenarios if they don't already exist.
154-154: LGTM: Buffer replacement in toProto methodThe change from
Buffer.from(data, 'base64')tobytesFromBase64(data)aligns with the PR objective of replacing Buffer usage. This new implementation should provide the same functionality.To ensure the correctness of this change, please verify that
bytesFromBase64handles the input correctly for all possible values ofdata. Consider adding unit tests to cover various scenarios if they don't already exist.
Line range hint
1-190: Overall assessment: Changes look good and align with PR objectivesThe modifications in this file successfully replace Buffer usage with polyfill functions
base64FromBytesandbytesFromBase64. These changes are consistent with the PR's objective of enhancing compatibility and performance.To ensure the robustness of these changes:
- Verify that the new polyfill functions handle all possible input types correctly.
- Consider adding or updating unit tests to cover various scenarios for both
fromProtoandtoProtomethods.- Ensure that these changes don't introduce any performance regressions, especially for large data sets.
src/core/move/msgs/MsgGovExecute.ts (4)
2-2: LGTM: New utility functions imported for base64 operationsThe addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility across different JavaScript environments.
140-140: LGTM: Replaced Buffer usage with polyfill in fromProto methodThe modification in the
fromProtomethod replaces the use ofBuffer.from(arg).toString('base64')with the importedbase64FromBytesfunction. This change aligns with the PR objective and improves cross-environment compatibility while maintaining the same functionality.
162-162: LGTM: Replaced Buffer usage with polyfill in toProto methodThe modification in the
toProtomethod replaces the use ofBuffer.from(arg, 'base64')with the importedbytesFromBase64function. This change aligns with the PR objective and improves cross-environment compatibility while maintaining the same functionality.
Line range hint
1-205: Overall assessment: Changes successfully implement the PR objectivesThe modifications in this file consistently replace Buffer usage with polyfill functions for base64 operations. These changes improve cross-environment compatibility without altering the core functionality of the
MsgGovExecuteclass. The implementation aligns well with the PR objectives of replacing the Buffer class with Uint8Array.src/core/wasm/msgs/MsgStoreAndMigrateContract.ts (2)
106-111: LGTM! Consistent use of new utility function.The changes in the
fromProtomethod effectively replace the use ofBufferwith the newbase64FromBytesutility function. This is applied consistently for bothdata.wasmByteCodeanddata.msg, aligning with the PR objective of enhancing compatibility and potentially improving performance.
121-124: LGTM! Consistent use of new utility function.The changes in the
toProtomethod effectively replace the use ofBufferwith the newbytesFromBase64utility function. This is applied consistently for bothwasm_byte_codeandmsg, maintaining symmetry with thefromProtomethod and aligning with the PR objective.src/core/wasm/msgs/MsgInstantiateContractV2.ts (2)
2-2: LGTM: Appropriate import for base64 conversion functionsThe addition of this import statement aligns well with the PR objective of replacing Buffer with more compatible alternatives. Importing specific functions (
base64FromBytesandbytesFromBase64) is a good practice for maintaining clean and efficient code.
Line range hint
1-180: Overall assessment: Changes successfully implement PR objectiveThe modifications in this file successfully replace Buffer usage with more compatible alternatives (
base64FromBytesandbytesFromBase64). The changes maintain the original functionality of theMsgInstantiateContractV2class while improving its compatibility and potentially its performance.Key points:
- Appropriate imports have been added.
- Both
fromProtoandtoProtomethods have been updated consistently.- The changes do not alter the class's public API, ensuring backward compatibility.
To ensure that all Buffer usages have been replaced in this file, you can run the following command:
If this command returns no results, it confirms that all Buffer usages have been successfully replaced.
src/core/auction/AuctionParams.ts (4)
2-2: LGTM: Import statement for polyfill utilities added.The import of
base64FromBytesandbytesFromBase64from the polyfill utility is consistent with the PR objective of replacing Buffer with Uint8Array. This change supports the implementation of base64 encoding and decoding operations without relying on Node.js-specific APIs.
119-119: LGTM: Buffer replaced with polyfill in fromProto method.The change from
Buffer.from(...).toString('base64')tobase64FromBytes(...)aligns with the PR objective. This modification maintains the same functionality while using a platform-agnostic approach, which enhances the portability of the code.
139-139: LGTM: Buffer replaced with polyfill in toProto method.The change from
Buffer.from(..., 'base64')tobytesFromBase64(...)is consistent with the PR objective and the previous change in thefromProtomethod. This modification ensures that the base64 to bytes conversion is handled in a platform-agnostic manner, improving the code's portability.
Line range hint
1-173: Overall assessment: Changes successfully implement Buffer replacement.The modifications in this file consistently replace Buffer usage with polyfill utilities for base64 encoding and decoding operations. These changes align well with the PR objectives, maintaining the original functionality while improving cross-platform compatibility. The code structure and readability are preserved, and the implementation appears consistent and error-free.
src/core/tx/SignatureV2.ts (2)
1-1: LGTM: Import statement forbytesFromBase64The import of
bytesFromBase64from the polyfill utility is consistent with the PR objective of replacing Buffer with Uint8Array. This change supports the goal of enhancing compatibility and performance within the codebase.
106-106: LGTM: Replacement of Buffer with bytesFromBase64The change from
Buffer.from(sigData.signature, 'base64')tobytesFromBase64(sigData.signature)aligns with the PR objective of replacing Buffer with Uint8Array. This modification enhances compatibility and potentially improves performance.Please verify the following:
- Ensure that the return type of the
toModeInfoAndSignaturemethod is updated if it has changed from[ModeInfo, Buffer]to[ModeInfo, Uint8Array].- Check for any other uses of Buffer in this file or related files that might need similar updates.
You can use the following script to check for other Buffer usages:
src/util/bcs.ts (3)
3-3: LGTM: New import aligns with PR objectives.The addition of
hexFromBytesandbytesFromHeximports from./polyfillis in line with the PR's goal of replacing theBufferclass withUint8Array. This change enhances compatibility and potentially improves performance.
Line range hint
3-24: Summary: Successfully replaced Buffer with polyfill functions.The changes in this file effectively replace the Node.js
Bufferclass with polyfill functions (bytesFromHexandhexFromBytes). These modifications align perfectly with the PR's objective of enhancing compatibility and potentially improving performance. The changes are consistent and well-implemented.Key improvements:
- Added import for polyfill functions.
- Updated input transformation to use
bytesFromHex.- Updated output transformation to use
hexFromBytes.These changes contribute to making the codebase more platform-agnostic and maintainable.
24-24: LGTM: Replaced Buffer with polyfill function in output transformation.The change from
`0x${Buffer.from(val).toString('hex')}`to`0x${hexFromBytes(val)}`is in line with the PR's objective of replacing theBufferclass. This modification improves cross-platform compatibility.To ensure consistency, let's verify that all
Buffer.from(...).toString('hex')calls have been replaced:#!/bin/bash # Search for any remaining Buffer.from(...).toString('hex') calls rg --type typescript "Buffer\.from\(.*\)\.toString\(['|\"]hex['|\"]\)"src/client/lcd/api/OphostAPI.ts (3)
9-9: LGTM: New import for polyfill utilities.The addition of
bytesFromHexandbase64FromBytesimports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility and performance.
154-154: LGTM: Replaced Buffer with polyfill utilities.The change from
Buffer.from(withdrawalHash, 'hex').toString('base64')tobase64FromBytes(bytesFromHex(withdrawalHash))successfully replaces the Buffer usage with the new polyfill utilities. This modification maintains the same functionality while improving compatibility and potentially performance.
Line range hint
1-190: Overall assessment: Changes align with PR objectives and improve code quality.The modifications in this file successfully replace Buffer usage with polyfill utilities, aligning with the PR's main objective. The changes enhance compatibility and potentially improve performance. Additionally, the update to
Number.parseInt()improves code consistency and adheres to modern JavaScript best practices.All changes have been reviewed and approved. The code quality has been improved without introducing any apparent issues or concerns.
src/core/ibc/core/connection/msgs/MsgConnectionOpenAck.ts (4)
2-2: LGTM: Import of polyfill utilities.The addition of
base64FromBytesandbytesFromBase64imports from the polyfill utility module aligns with the PR objective of replacing Buffer usage. This change enhances cross-platform compatibility.
119-121: LGTM: Consistent replacement of Buffer with base64FromBytes.The changes in the
fromProtomethod successfully replace Buffer usage with thebase64FromBytesutility function for all proof data conversions. This modification aligns with the PR objective and maintains the existing logic while improving cross-platform compatibility.
148-150: LGTM: Consistent replacement of Buffer with bytesFromBase64.The changes in the
toProtomethod successfully replace Buffer usage with thebytesFromBase64utility function for all proof data conversions. This modification aligns with the PR objective and maintains the existing logic while improving cross-platform compatibility.
Line range hint
1-190: Summary: Successful replacement of Buffer with polyfill utilities.The changes in this file consistently replace Buffer usage with the new polyfill utility functions
base64FromBytesandbytesFromBase64. These modifications:
- Align with the PR objective of replacing the Buffer class with Uint8Array.
- Enhance cross-platform compatibility.
- Maintain the existing logic and functionality of the
MsgConnectionOpenAckclass.- Are minimal and focused, reducing the risk of introducing new bugs.
The implementation is clean and consistent throughout the file.
src/core/ophost/BridgeConfig.ts (1)
2-2: LGTM: Import statement for polyfill functions.The import of
base64FromBytesandbytesFromBase64from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility across different JavaScript environments.src/core/ibc/core/connection/msgs/MsgConnectionOpenTry.ts (4)
130-132: LGTM! Consistent use of polyfill function.The replacement of
Buffer.from()withbase64FromBytesis consistent with the PR objective and the earlier import changes. This modification enhances cross-environment compatibility.
161-163: LGTM! Consider adding tests for the new implementation.The use of
bytesFromBase64instead ofBuffer.from()is consistent with the PR objective and earlier changes. This modification completes the transition away from the Buffer class in this file.To ensure the new implementation works as expected, please run the following script to check for existing tests or suggest adding new ones:
#!/bin/bash # Description: Check for tests covering the MsgConnectionOpenTry class and its methods. # Test: Search for test files related to MsgConnectionOpenTry fd -e test.ts -e spec.ts MsgConnectionOpenTry # Test: If test files exist, check for specific test cases for toProto method rg --type typescript 'describe.*MsgConnectionOpenTry.*toProto' $(fd -e test.ts -e spec.ts MsgConnectionOpenTry)If no tests are found, consider adding unit tests to verify the correct functioning of the
toProtomethod with the newbytesFromBase64implementation.
80-82: LGTM! Consider data validation.The simplification of proof assignments aligns with the PR objective and improves code readability. However, ensure that the input data is properly validated before reaching this point to maintain data integrity.
To verify the data validation, please run the following script:
#!/bin/bash # Description: Check for data validation of proof fields before they reach the fromData method. # Test: Search for validation of proof_init, proof_client, and proof_consensus rg --type typescript -A 5 'proof_init|proof_client|proof_consensus.*validate'
2-2: LGTM! Verify polyfill implementation.The addition of
base64FromBytesandbytesFromBytesfrom the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change enhances compatibility across different JavaScript environments.To ensure the polyfill functions are correctly implemented, please run the following script:
src/key/Key.ts (7)
14-14: LGTM: New utility functions importedThe new import statement adds necessary utility functions for byte operations, aligning with the PR objective of replacing Buffer with Uint8Array.
91-91: LGTM: UpdatedcreateSignatureAminoimplementationThe change replaces Buffer operations with new utility functions, maintaining the same logic while aligning with the use of Uint8Array. This is consistent with the PR objective.
120-120: LGTM: UpdatedcreateSignatureimplementationThe change replaces Buffer operations with the new utility function, maintaining the same logic while aligning with the use of Uint8Array. This is consistent with the PR objective.
147-151: LGTM: UpdatedcreateSignatureEIP191implementationThe changes replace Buffer operations with new utility functions and
concatBytes, maintaining the same logic while aligning with the use of Uint8Array. This is consistent with the PR objective and improves compatibility.Also applies to: 160-160
Line range hint
1-214: Overall assessment: Changes are consistent and well-implementedThe modifications in this file successfully replace Buffer with Uint8Array and introduce appropriate utility functions. These changes align perfectly with the PR objective of enhancing compatibility and performance. The consistency in implementation across different methods is commendable.
However, it's crucial to ensure that these changes are reflected throughout the codebase, particularly in areas that interact with the modified methods. Consider running comprehensive tests to verify the integrity of the system after these changes.
38-38: LGTM: Updated method signature forsignWithKeccak256The change from
BuffertoUint8Arrayaligns with the PR objective. However, ensure that all calls to this method across the codebase have been updated accordingly.To verify the usage of this method across the codebase, run the following script:
✅ Verification successful
The
signWithKeccak256method is only used withinsrc/key/Key.ts, and all usages are consistent with the updated signature. No additional changes are required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to the 'signWithKeccak256' method rg --type typescript -e '\.signWithKeccak256\s*\(' -A 3Length of output: 92
Script:
#!/bin/bash # Search for calls to the 'signWithKeccak256' method using the correct TypeScript file type rg --type ts -e '\.signWithKeccak256\s*\(' -A 3Length of output: 213
30-30: LGTM: Updated method signature forsignThe change from
BuffertoUint8Arrayaligns with the PR objective. However, ensure that all calls to this method across the codebase have been updated accordingly.To verify the usage of this method across the codebase, run the following script:
✅ Verification successful
Verification Complete: Method signature for
signis consistently updatedAll usages of the
signmethod insrc/key/Key.tsalign with the updated signature. Thethis.app.signcall insrc/key/ledger/LedgerKey.tsappears to reference a different method and is unaffected by this change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to the 'sign' method rg --type typescript -e '\.sign\s*\(' -A 3Length of output: 79
Script:
#!/bin/bash # Search for calls to the 'sign' method rg --type ts -e '\.sign\s*\(' -A 3Length of output: 763
src/core/ophost/msgs/MsgFinalizeTokenWithdrawal.ts (4)
2-2: LGTM: Import of base64 utility functions.The addition of
base64FromBytesandbytesFromBase64imports aligns with the PR objective of replacing Buffer with Uint8Array. These utility functions will be used for base64 encoding and decoding operations throughout the class.
180-187: LGTM: Consistent use of base64FromBytes utility function.The changes in the
fromProtomethod successfully replace Buffer usage with the newbase64FromBytesutility function. This modification:
- Aligns with the PR objective of replacing Buffer with Uint8Array.
- Improves code consistency across the class.
- Maintains the same logic while updating the implementation method.
The changes look good and achieve the intended goal.
210-217: LGTM: Consistent use of bytesFromBase64 utility function.The changes in the
toProtomethod successfully replace Buffer usage with the newbytesFromBase64utility function. This modification:
- Aligns with the PR objective of replacing Buffer with Uint8Array.
- Improves code consistency across the class.
- Maintains the same logic while updating the implementation method.
The changes look good and achieve the intended goal.
Line range hint
1-287: Consider updating related tests and documentation.The changes to
MsgFinalizeTokenWithdrawalclass look good and consistently implement the replacement of Buffer with Uint8Array using the new utility functions. To ensure completeness:
- Please update any unit tests related to this class to reflect the new implementation.
- If there's any documentation (e.g., JSDoc comments, README, or separate documentation files) mentioning the use of Buffer in this context, please update it to reflect the new implementation using Uint8Array and the utility functions.
This will help maintain consistency across the codebase and prevent confusion for other developers.
To help verify the test coverage, you can run the following command:
src/core/PublicKey.ts (6)
117-117: LGTM! Consistent with PR objective.The change from
Buffer.from(...).toString('base64')tobase64FromBytesis in line with the PR objective of replacing Buffer with Uint8Array-based utilities. The added return type improves type safety.
122-122: LGTM! Consistent with PR objective.The change from
Buffer.from(..., 'base64')tobytesFromBase64aligns with the PR objective of replacing Buffer with Uint8Array-based utilities. The added return type enhances type safety.
138-138: LGTM! Efficient use of concatBytes.The change from
Buffer.concattoconcatBytesis consistent with the PR objective and provides an efficient way to combine Uint8Arrays. This change maintains the functionality while aligning with the new Uint8Array-based approach.
142-143: LGTM! Consistent use of bytesFromBase64.The change from
Buffer.from(..., 'base64')tobytesFromBase64is in line with the PR objective and provides a consistent way to convert base64 strings to Uint8Array. This change maintains the functionality while aligning with the new Uint8Array-based approach.
325-325: LGTM! Consistent with PR objective.The change from
Buffer.from(...).toString('base64')tobase64FromBytesaligns with the PR objective of replacing Buffer with Uint8Array-based utilities. The added return type improves type safety.
330-330: LGTM! Consistent use of bytesFromBase64.The changes from
Buffer.from(..., 'base64')tobytesFromBase64in bothtoProtoandrawAddressmethods are in line with the PR objective. These changes provide a consistent way to convert base64 strings to Uint8Array while maintaining the original functionality.Also applies to: 350-351
src/client/lcd/api/TxAPI.ts (4)
15-16: LGTM: Import changes align with PR objectivesThe updated import for
hashToHexand the new imports for base64 conversion functions are in line with the PR's goal of replacing Buffer with Uint8Array. These changes should improve compatibility and performance.
376-376: LGTM: Encode method updated to use polyfillThe
encodemethod has been successfully updated to use thebase64FromBytespolyfill function, replacing the previous Buffer-based implementation. This change aligns with the PR's objective and should improve cross-platform compatibility.
384-384: LGTM: Decode method updated to use polyfillThe
decodemethod has been successfully updated to use thebytesFromBase64polyfill function, replacing the previous Buffer-based implementation. This change is consistent with the PR's objective and should enhance cross-platform compatibility.
Line range hint
1-624: Overall assessment: Changes successfully implement PR objectivesThe modifications in this file effectively replace Buffer usage with Uint8Array polyfills in the
encodeanddecodemethods. These changes align perfectly with the PR's objective of enhancing compatibility and performance. The public API remains unchanged, ensuring backward compatibility. No new issues or potential bugs were introduced. The implementation is clean and focused.src/core/ibc/core/client/msgs/tendermint/types.ts (7)
297-301: Approve BlockID class changes.The renaming of
partSetHeadertopart_set_headeris consistent with the new naming convention applied throughout the file. The changes have been applied consistently across all methods of the BlockID class.Also applies to: 316-319, 324-327, 334-345
387-387: Approve PartSetHeader class changes.The updates to base64 handling in the PartSetHeader class, using the new utility functions
base64FromBytesandbytesFromBase64, are consistent with changes made throughout the file and align with the PR objective.Also applies to: 400-400, 407-407
426-432: Approve Commit class changes.The renaming of
blockIdtoblock_idin the Commit class is consistent with the new naming convention applied throughout the file. The changes have been applied consistently across all methods of the Commit class.Also applies to: 448-452, 458-462, 479-483
507-514: Approve CommitSig class changes.The renaming of properties and the updates to base64 handling in the CommitSig class are consistent with the changes made throughout the file. These changes adhere to the new naming convention and align with the PR objective of replacing Buffer with Uint8Array.
Also applies to: 541-544, 555-557, 562-569
593-598: Approve ValidatorSet class changes.The renaming of
totalVotingPowertototal_voting_powerin the ValidatorSet class is consistent with the new naming convention applied throughout the file. The changes have been applied consistently across all methods of the ValidatorSet class.Also applies to: 622-626, 641-645
667-675: Approve Validator class changes and suggest final verification.The renaming of properties and the updates to base64 handling in the Validator class are consistent with the changes made throughout the file. These changes adhere to the new naming convention and align with the PR objective of replacing Buffer with Uint8Array.
To ensure all changes have been applied consistently, run the following script:
Also applies to: 690-695, 700-705, 713-726
153-161: Approve base64 handling changes and suggest performance testing.The replacement of Buffer with
base64FromBytesandbytesFromBase64functions aligns with the PR objective and should improve cross-platform compatibility.Consider running performance tests to compare the efficiency of the new implementation against the old one:
Also applies to: 188-196
src/core/tx/CompactBitArray.ts (7)
2-2: Imports updated to use polyfill functionsThe
base64FromBytesandbytesFromBase64functions are correctly imported from../../util/polyfill, replacing Node.jsBufferdependencies for base64 operations.
7-7: Updated 'elems' type to 'Uint8Array'Changing
elemstype toUint8Arrayaligns with the transition fromBuffer, enhancing compatibility with browser environments and improving performance.
23-23: Initialized 'elems' with 'Uint8Array'Replaced
Buffer.alloc(num_elems)withnew Uint8Array(num_elems), which effectively creates a zero-filled array of the required length.
42-42: Direct element access in 'getIndex' methodAccessing
this.elems[i >> 3]directly is appropriate withUint8Array, simplifying the code by eliminating the need forreadUInt8.
86-86: Used 'bytesFromBase64' for base64 decodingIn the
fromDatamethod, replacingBuffer.from(data.elems, 'base64')withbytesFromBase64(data.elems)correctly decodes the base64 string into aUint8Array.
92-92: Used 'base64FromBytes' for base64 encodingUpdated
this.elems.toString('base64')tobase64FromBytes(this.elems)in thetoDatamethod, ensuring proper encoding ofUint8Arrayto a base64 string without relying onBuffer.
98-98: Simplified 'fromProto' methodDirectly assigning
proto.elemstoelemsis appropriate sinceproto.elemsis already aUint8Array, streamlining the method and removing unnecessary conversions.src/core/bech32.ts (3)
2-2: Importing polyfill functions enhances compatibilityGreat job replacing the Node.js
Bufferclass withbytesFromHexandhexFromBytesfrom'../util/polyfill'. This improves cross-platform compatibility and aligns with the PR objectives.
86-88: Consistent use ofUint8ArrayintoBytesfunctionRenaming
toBuffertotoBytesand changing the return type toUint8Arrayaligns with modern JavaScript practices and enhances compatibility. This change is consistent with the PR's objective to replaceBufferusage.
62-62: Verify correctness of address conversion intoHexThe
toHexfunction now converts the bech32 address to a hexadecimal string usinghexFromBytes. Ensure that thevals.wordscorrectly represent the original address bytes and that the conversion handles all edge cases appropriately.To confirm the correctness, you can run the following script to test address conversions:
src/core/opchild/msgs/MsgFinalizeTokenDeposit.ts (2)
2-2: Approved: ReplacingBufferwith base64 utility functionsThe import of
base64FromBytesandbytesFromBase64from../../../util/polyfilleffectively replaces the use ofBuffer, enhancing compatibility and aligning with the pull request objectives.
133-133: Approved: Correct conversion ofdatausingbytesFromBase64In the
toProtomethod, the use ofbytesFromBase64(data)properly replaces the previousBufferimplementation, ensuring data is accurately converted from a base64 string to bytes.src/core/slashing/SlashingParams.ts (3)
2-2: Importing UTF-8 conversion utilitiesThe addition of
utf8FromBytesandbytesFromUtf8from'../../util/polyfill'correctly replaces the use ofBufferand enhances compatibility.
110-113: Updated deserialization using UTF-8 conversionThe use of
Number.parseFloat(utf8FromBytes(...))forminSignedPerWindow,slashFractionDoubleSign, andslashFractionDowntimein thefromProtomethod correctly decodes the byte arrays and parses them as floats. This change aligns with the removal ofBufferusage and ensures proper deserialization.
128-133: Updated serialization using UTF-8 conversionSerializing
min_signed_per_window,slash_fraction_double_sign, andslash_fraction_downtimeusingbytesFromUtf8in thetoProtomethod ensures these numerical values are correctly encoded as UTF-8 byte arrays. This modification appropriately replaces the previousBufferimplementation and maintains data integrity during serialization.src/core/wasm/msgs/MsgStoreAndInstantiateContract.ts (3)
184-184: Utilizebase64FromBytesfor base64 encoding infromProtoReplacing
Buffermethods withbase64FromBytesfor encodingdata.wasmByteCode,data.msg, anddata.codeHashensures consistent base64 encoding without relying on Node.js-specific features.Also applies to: 191-191, 195-195
216-216: UsebytesFromBase64for base64 decoding intoProtoSubstituting
Buffermethods withbytesFromBase64for decodingwasm_byte_code,msg, andcode_hashmaintains functionality while improving cross-platform compatibility.Also applies to: 221-221, 225-225
Line range hint
2-225: Verify the robustness of base64 utility functionsEnsure that
base64FromBytesandbytesFromBase64correctly handle all edge cases, such as empty inputs, large data arrays, and invalid base64 strings, to prevent potential runtime errors.src/core/tx/Tx.ts (4)
76-76: LGTM!The use of
base64FromBytesin thefromProtomethod correctly replaces the Node.jsBuffermethod, ensuring consistency in base64 encoding throughout the codebase.
84-84: LGTM!The update to use
bytesFromBase64in thetoProtomethod appropriately handles base64 decoding, aligning with the updated utility functions and removing the dependency onBuffer.
142-142: LGTM!The use of
base64FromBytesin theappendSignaturesmethod ensures consistent base64 encoding of signatures and aligns with the updated encoding practices without relying on Node.jsBuffer.
92-92:⚠️ Potential issueUpdate all references to
fromBufferThe static method
fromBufferhas been renamed tofromBytes, and the parameter type has changed fromBuffertoUint8Array. Ensure that all references toTx.fromBufferin the codebase are updated toTx.fromBytesto maintain consistency and prevent runtime errors.
| params | ||
| ) | ||
| .then((d) => ({ | ||
| num_of_accounts: parseInt(d.num_of_accounts), | ||
| num_of_forwards: parseInt(d.num_of_forwards), | ||
| num_of_accounts: Number.parseInt(d.num_of_accounts), | ||
| num_of_forwards: Number.parseInt(d.num_of_forwards), | ||
| total_forwarded: Coin.fromData(d.total_forwarded), | ||
| })) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for parsing operations.
While the current implementation works well for valid input, it might be beneficial to add error handling for cases where the parsed strings are not valid numbers. This would make the code more robust and prevent potential runtime errors.
Here's a suggested implementation with error handling:
public async stats(
channel: string,
params: APIParams = {}
): Promise<ForwardingStats> {
return this.c
.get<ForwardingStats.Data>(
`/noble/forwarding/v1/stats/${channel}`,
params
)
.then((d) => {
const num_of_accounts = Number.parseInt(d.num_of_accounts, 10);
const num_of_forwards = Number.parseInt(d.num_of_forwards, 10);
if (isNaN(num_of_accounts) || isNaN(num_of_forwards)) {
throw new Error('Invalid data: num_of_accounts or num_of_forwards is not a number');
}
return {
num_of_accounts,
num_of_forwards,
total_forwarded: Coin.fromData(d.total_forwarded),
};
});
}This implementation adds checks for NaN values after parsing and throws an error if invalid data is encountered.
| } | ||
|
|
||
| public toProto(): AcceptedMessagesFilter.Proto { | ||
| return AcceptedMessagesFilter_pb.fromPartial({ | ||
| messages: this.messages.map((msg) => Buffer.from(msg, 'base64')), | ||
| messages: this.messages.map(bytesFromBase64), |
There was a problem hiding this comment.
Potential issue: Incorrect utility function used for base64 encoding.
While the change aligns with the PR objective of removing Buffer usage, there appears to be a logical error in the implementation. The bytesFromBase64 function is being used where we expect base64 encoding to occur. This is likely to cause incorrect behavior.
Consider replacing bytesFromBase64 with base64FromBytes to maintain the correct encoding direction:
- messages: this.messages.map(bytesFromBase64),
+ messages: this.messages.map(base64FromBytes),Please verify if this change maintains the intended functionality of the toProto method.
📝 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.
| messages: this.messages.map(bytesFromBase64), | |
| messages: this.messages.map(base64FromBytes), |
| @@ -1,4 +1,5 @@ | |||
| import { JSONSerializable } from '../../../../../util/json' | |||
| import { base64FromBytes, bytesFromBase64 } from '../../../../../util/polyfill' | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing Export: bytesFromBase64 in Polyfill Utility
The polyfill.ts file correctly exports base64FromBytes, aligning with the PR objective. However, the bytesFromBase64 function is not found among the exports, which may lead to runtime issues where this function is expected.
- File:
src/util/polyfill.ts- Missing Export:
bytesFromBase64
- Missing Export:
Please ensure that bytesFromBase64 is properly exported in the polyfill.ts file.
🔗 Analysis chain
LGTM! Verify the import path.
The addition of base64FromBytes and bytesFromBase64 imports from the polyfill utility aligns with the PR objective of replacing Buffer with Uint8Array. This change should improve compatibility and potentially performance.
To ensure the import path is correct, run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the polyfill utility file
# Test: Check if the polyfill utility file exists
fd -p "src/.*util/polyfill.(ts|js)" --exec echo "Polyfill utility file found: {}"
# Test: Verify the exported functions in the polyfill utility
rg -t typescript "export.*(base64FromBytes|bytesFromBase64)" -g "src/**/util/polyfill.ts"
Length of output: 377
Script:
#!/bin/bash
# Description: Verify the existence of the polyfill utility file
# Test: Check if the polyfill utility file exists
fd "src/**/util/polyfill.(ts|js)" --exec echo "Polyfill utility file found: {}"
# Test: Verify the exported functions in the polyfill utility
rg --type=typescript "export.*(base64FromBytes|bytesFromBase64)" -g "src/**/util/polyfill.ts"
Length of output: 212
Script:
#!/bin/bash
# Description: Verify the existence of the polyfill utility file
# Test: Check if the polyfill utility file exists
fd "polyfill.ts" "polyfill.js" src/**/util/ --exec echo "Polyfill utility file found: {}"
# Test: Verify the exported functions in the polyfill utility
rg "export\s+(function\s+base64FromBytes|const\s+bytesFromBase64)" "src/**/util/polyfill.ts" "src/**/util/polyfill.js"
Length of output: 496
Script:
#!/bin/bash
# Description: Verify the existence of the polyfill utility file
# Test: Check if the polyfill utility file exists
fd -e ts -e js "polyfill\.(ts|js)$" src/core/ibc/core/channel/util/ --exec echo "Polyfill utility file found: {}"
# Test: Verify the exported functions in the polyfill utility
rg "export\s+(function\s+base64FromBytes|const\s+bytesFromBase64)" src/core/ibc/core/channel/util/polyfill.ts src/core/ibc/core/channel/util/polyfill.js
Length of output: 555
Script:
#!/bin/bash
# Description: Globally verify the existence of the polyfill utility files and their exports
# Test: Check if any polyfill utility file exists
fd "polyfill\.(ts|js)$" --type f --exec echo "Polyfill utility file found: {}"
# Capture the found files into a variable
polyfill_files=$(fd "polyfill\.(ts|js)$" --type f)
# Check if any polyfill files were found
if [ -z "$polyfill_files" ]; then
echo "No polyfill utility files found."
exit 1
fi
# Test: Verify the exported functions in each polyfill utility file
for file in $polyfill_files; do
echo "Checking exports in $file"
rg "export\s+(function\s+base64FromBytes|const\s+bytesFromBase64)" "$file"
done
Length of output: 510
| @@ -18,9 +19,9 @@ const initiaAddress = ( | |||
| throw new Error('invalid address') | |||
| } | |||
|
|
|||
| return Buffer.from(val, 'hex') | |||
| return bytesFromHex(val) | |||
There was a problem hiding this comment.
💡 Codebase verification
Remaining Buffer.from(..., 'hex') Call Found
One instance of Buffer.from(..., 'hex') remains in src/util/polyfill.ts:
return Uint8Array.from(gt.Buffer.from(hex, 'hex'))
🔗 Analysis chain
LGTM: Replaced Buffer with polyfill function.
The change from Buffer.from(val, 'hex') to bytesFromHex(val) aligns with the PR's goal of replacing the Buffer class. This modification enhances cross-platform compatibility.
To ensure consistency, let's verify that all Buffer.from(..., 'hex') calls have been replaced:
Action Required: Verify all instances of Buffer.from(..., 'hex') have been replaced.
The previous search command failed to recognize the typescript file type. Let's retry the search without specifying the file type to ensure all files are scanned:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining Buffer.from(..., 'hex') calls
rg --type typescript "Buffer\.from\(.*,\s*['|\"]hex['|\"]\)"
Length of output: 111
Script:
#!/bin/bash
# Search for any remaining Buffer.from(..., 'hex') calls without specifying file type
rg "Buffer\.from\(.*,\s*['|\"]hex['|\"]\)"
Length of output: 124
| public chain_id: string, | ||
| public height: string, | ||
| public time: Date | undefined, | ||
| public lastBlockId: BlockID | undefined, | ||
| public lastCommitHash: string, | ||
| public dataHash: string, | ||
| public validatorsHash: string, | ||
| public nextValidatorsHash: string, | ||
| public consensusHash: string, | ||
| public appHash: string, | ||
| public lastResultsHash: string, | ||
| public evidenceHash: string, | ||
| public proposerAddress: string | ||
| public last_block_id: BlockID | undefined, | ||
| public last_commit_hash: string, | ||
| public data_hash: string, | ||
| public validators_hash: string, | ||
| public next_validators_hash: string, | ||
| public consensus_hash: string, | ||
| public app_hash: string, | ||
| public last_results_hash: string, | ||
| public evidence_hash: string, | ||
| public proposer_address: string |
There was a problem hiding this comment.
💡 Codebase verification
Property renaming incomplete; old camelCase usages found.
The renaming from camelCase to snake_case is not fully propagated. The old property names are still used in the following files:
src/core/ibc/core/client/msgs/tendermint/types.tssrc/client/lcd/LCDClient.tssrc/client/lcd/Wallet.tssrc/core/tx/SignDoc.tssrc/key/Key.tssrc/client/LocalInitia.ts
Please update these references to use the new snake_case property names to ensure consistency and prevent potential issues.
🔗 Analysis chain
Approve property renaming and suggest verification.
The consistent renaming of properties from camelCase to snake_case improves adherence to a more Python-like naming convention. This change enhances readability for developers familiar with this style.
To ensure this change doesn't break existing code, please run the following script to check for any remaining uses of the old property names:
Also applies to: 62-74, 78-90, 97-109, 113-125, 153-161, 168-180, 184-196
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining uses of the old camelCase property names
rg -t ts 'chainId|lastBlockId|lastCommitHash|dataHash|validatorsHash|nextValidatorsHash|consensusHash|appHash|lastResultsHash|evidenceHash|proposerAddress' src/
Length of output: 3802
| export function concatBytes(arrs: Uint8Array[]): Uint8Array { | ||
| if (gt.Buffer) { | ||
| return gt.Buffer.concat(arrs) | ||
| } else { | ||
| return arrs.reduce((acc, arr) => { | ||
| const combined = new Uint8Array(acc.length + arr.length) | ||
| combined.set(acc) | ||
| combined.set(arr, acc.length) | ||
| return combined | ||
| }, new Uint8Array()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize array concatenation performance
The current implementation of concatBytes in the absence of gt.Buffer uses repeated array allocations, which can be inefficient for large arrays. Consider precomputing the total length and allocating a single Uint8Array to improve performance.
Refactor the code as follows:
const totalLength = arrs.reduce((sum, arr) => sum + arr.length, 0)
const combined = new Uint8Array(totalLength)
let offset = 0
arrs.forEach((arr) => {
combined.set(arr, offset)
offset += arr.length
})
return combined| if (typeof global !== 'undefined') { | ||
| return global | ||
| } | ||
| throw 'Unable to locate global object' |
There was a problem hiding this comment.
Use Error object when throwing exceptions
Throwing a string literal does not provide a stack trace and is less informative. Replace the string with a new Error object to improve error handling and debugging.
Apply this diff to fix the issue:
- throw 'Unable to locate global object'
+ throw new Error('Unable to locate global object')📝 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.
| throw 'Unable to locate global object' | |
| throw new Error('Unable to locate global object') |
| } else { | ||
| const bin: string[] = [] | ||
| arr.forEach((byte) => { | ||
| bin.push(gt.String.fromCharCode(byte)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use String.fromCharCode directly instead of gt.String.fromCharCode
String is a global constructor and doesn't need to be accessed via gt. Using String.fromCharCode improves readability and avoids potential issues if gt.String is undefined.
Apply this diff to simplify the code:
- bin.push(gt.String.fromCharCode(byte))
+ bin.push(String.fromCharCode(byte))📝 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.
| bin.push(gt.String.fromCharCode(byte)) | |
| bin.push(String.fromCharCode(byte)) |
| bech32.toWords(bytesFromHex(hex.padStart(40, '0'))) | ||
| ) | ||
| } else { | ||
| return bech32.encode( | ||
| 'init', | ||
| bech32.toWords(Buffer.from(hex.padStart(64, '0'), 'hex')) | ||
| bech32.toWords(bytesFromHex(hex.padStart(64, '0'))) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor fromHex to eliminate code duplication
The fromHex function contains duplicated logic for handling different hex lengths. Consider refactoring to simplify the code and improve maintainability.
Apply this diff to refactor the function:
export function fromHex(hexAddress: string): AccAddress {
const hex = hexAddress.replace(/^0x0+|^0x|^0+(?!x)/, '')
- if (hex.length <= 40) {
- return bech32.encode(
- 'init',
- bech32.toWords(bytesFromHex(hex.padStart(40, '0')))
- )
- } else {
- return bech32.encode(
- 'init',
- bech32.toWords(bytesFromHex(hex.padStart(64, '0')))
- )
- }
+ const paddedHex = hex.padStart(hex.length <= 40 ? 40 : 64, '0')
+ return bech32.encode('init', bech32.toWords(bytesFromHex(paddedHex)))
}📝 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.
| bech32.toWords(bytesFromHex(hex.padStart(40, '0'))) | |
| ) | |
| } else { | |
| return bech32.encode( | |
| 'init', | |
| bech32.toWords(Buffer.from(hex.padStart(64, '0'), 'hex')) | |
| bech32.toWords(bytesFromHex(hex.padStart(64, '0'))) | |
| const paddedHex = hex.padStart(hex.length <= 40 ? 40 : 64, '0') | |
| return bech32.encode('init', bech32.toWords(bytesFromHex(paddedHex))) | |
| ) | |
| } |
| msgProto.data.length | ||
| ? Buffer.from(msgProto.data).toString('base64') | ||
| : undefined | ||
| msgProto.data.length ? base64FromBytes(msgProto.data) : undefined |
There was a problem hiding this comment.
Handle undefined msgProto.data before accessing length
In the fromProto method, the expression msgProto.data.length assumes that msgProto.data is always defined. If msgProto.data is undefined, accessing length will result in a runtime error. Please add a check to ensure msgProto.data is defined before accessing its length property.
Suggested change:
- msgProto.data.length ? base64FromBytes(msgProto.data) : undefined
+ msgProto.data && msgProto.data.length ? base64FromBytes(msgProto.data) : undefined📝 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.
| msgProto.data.length ? base64FromBytes(msgProto.data) : undefined | |
| msgProto.data && msgProto.data.length ? base64FromBytes(msgProto.data) : undefined |
|
closing this as initia.js has many external dependencies that have issues with polyfill |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores