Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decode TxData (Singular & Multi) #66

Merged
merged 3 commits into from
Oct 2, 2024
Merged

Decode TxData (Singular & Multi) #66

merged 3 commits into from
Oct 2, 2024

Conversation

bh2smith
Copy link
Collaborator

@bh2smith bh2smith commented Oct 2, 2024

User description

This is a front end helper function that transforms UserOperation into something a bit more human readable.

Test Plan

See added unit tests


PR Type

Enhancement, Tests


Description

  • Added isMultisendTx function in src/lib/multisend.ts to identify multisend transactions.
  • Implemented decodeTxData method in NearSafe class to decode transaction data and determine transaction type.
  • Updated types in src/types.ts to include EvmTransactionData and modified EncodedTxData to use it.
  • Added unit tests for decodeTxData method in NearSafe class.
  • Added ethers-multisend dependency in package.json.

Changes walkthrough 📝

Relevant files
Enhancement
multisend.ts
Add function to identify multisend transactions                   

src/lib/multisend.ts

  • Added isMultisendTx function to check if a transaction is a multisend
    transaction.
  • +8/-0     
    near-safe.ts
    Add and integrate transaction data decoding functionality

    src/near-safe.ts

  • Imported decodeMulti and decodeFunctionData from dependencies.
  • Added decodeTxData method to decode transaction data.
  • Utilized isMultisendTx to determine transaction type.
  • +44/-2   
    types.ts
    Update types for EVM transaction data                                       

    src/types.ts

  • Added EvmTransactionData interface.
  • Updated EncodedTxData to use EvmTransactionData.
  • +9/-6     
    Tests
    nearsafe.spec.ts
    Add unit test for transaction data decoding                           

    tests/unit/nearsafe.spec.ts

    • Added unit test for decodeTxData method in NearSafe class.
    +42/-0   
    Dependencies
    package.json
    Add ethers-multisend dependency                                                   

    package.json

    • Added ethers-multisend dependency.
    +1/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @mintbase-codium-pr-agent
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Error Handling
    The method decodeTxData in NearSafe class assumes data.data can always be parsed to UserOperation. This might not always be the case, and it could lead to runtime errors if the parsing fails. Consider adding error handling for cases where data.data does not conform to the expected format.

    Hardcoded Values
    The method decodeTxData uses hardcoded values to determine if a transaction is a multisend transaction (isMultisendTx(args)). This could limit flexibility and maintainability. Consider externalizing these values or providing a mechanism to configure them.

    @mintbase-codium-pr-agent
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add a type guard to ensure safe casting of args[0] to string

    Consider adding a type guard to ensure that args[0] can be safely cast to string.
    This will prevent runtime errors if args[0] is not a string.

    src/lib/multisend.ts [63]

    -const to = (args[0] as string).toLowerCase();
    +if (typeof args[0] !== 'string') throw new TypeError('Expected string for args[0]');
    +const to = args[0].toLowerCase();
     
    Suggestion importance[1-10]: 10

    Why: Adding a type guard to ensure args[0] is a string before casting prevents potential runtime errors, which is crucial for robust code.

    10
    Add error handling for JSON parsing to prevent runtime exceptions

    Handle potential exceptions when parsing data.data with JSON.parse to avoid runtime
    errors if data.data is not a valid JSON string.

    src/near-safe.ts [261]

    -const userOp: UserOperation = JSON.parse(data.data);
    +let userOp: UserOperation;
    +try {
    +  userOp = JSON.parse(data.data);
    +} catch (error) {
    +  throw new Error('Failed to parse data.data as JSON');
    +}
     
    Suggestion importance[1-10]: 10

    Why: Handling potential exceptions when parsing JSON is essential to avoid runtime errors, making the code more reliable and robust.

    10
    Possible issue
    Validate the structure of userOp to ensure all required fields are present

    Consider validating the structure of userOp after parsing to ensure it contains all
    required fields before accessing them.

    src/near-safe.ts [262]

    +const requiredFields = ['callGasLimit', 'maxFeePerGas', 'maxPriorityFeePerGas'];
    +for (const field of requiredFields) {
    +  if (!(field in userOp)) {
    +    throw new Error(\`Missing required field \${field}\`);
    +  }
    +}
     const { callGasLimit, maxFeePerGas, maxPriorityFeePerGas } = userOp;
     
    Suggestion importance[1-10]: 9

    Why: Validating the structure of userOp ensures that all required fields are present before accessing them, preventing potential runtime errors and improving code reliability.

    9
    Enhancement
    Ensure accurate arithmetic operations with large numbers using BigInt

    Use BigInt for arithmetic operations on callGasLimit and maxGasPrice to ensure
    accurate calculations with large numbers.

    src/near-safe.ts [283]

    -costEstimate: formatEther(BigInt(callGasLimit) * maxGasPrice),
    +costEstimate: formatEther(BigInt(callGasLimit) * BigInt(maxGasPrice)),
     
    Suggestion importance[1-10]: 8

    Why: Using BigInt for all arithmetic operations involving large numbers ensures accuracy, which is important for financial calculations.

    8

    @bh2smith bh2smith merged commit f9e4b0e into main Oct 2, 2024
    1 check passed
    @bh2smith bh2smith deleted the decode-user-op branch October 2, 2024 13:29
    @bh2smith bh2smith changed the title Decode TxData Singular Decode TxData (Singular & Multi) Oct 2, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant