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

feat: let prettyLogTransaction function to support address mapping #39

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alan890104
Copy link

@alan890104 alan890104 commented Jan 7, 2024

Description

As a Tact / funC smart contract developers, when I want to conduct a deeper check if the test failed, I would like to use prettyLogTransactions to show the TON transfer flow. However, the output of the original prettyLogTransactions contains only addresses, in some scenarios that more than 10 addresses involved in is indeed hard to debug. Thus, I wish to have a optional address mapping mechanism to help me tagging those addresses to be easily to understand with a nickname.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Build succeeds locally with my changes

@krigga
Copy link
Collaborator

krigga commented Feb 6, 2024

Hello, this is a nice suggestion, however I think a different implementation is in order.
It might be more useful to have the mapping defined as a function in the first place, instead of a Map - other users may prefer a different approach, and it is possible to use a Map in the functional mapper. Or perhaps both a function or a Map should be supported as a mapping.
Also, please do not use the Maybe type from @ton/core since it is not exported. Instead, use it's definition (I think it's T | null | undefined), or check if it is exported in a newer version of @ton/core than is indicated in the current devDependencies and if it is so, let me know.

@alan890104
Copy link
Author

Summary:

This PR introduces enhancements to the transaction logging functionality by adding support for optional address and operation code (op) mapping. These changes allow developers to provide custom mapping functions to make transaction logs more human-readable without altering the underlying data.

Changes Introduced:

  1. Address Mapping in prettyLogTransaction:

    • An optional AddressMapFunc is added to map src and dest addresses to human-readable formats.
    • If mapFunc is not provided, or if the mapFunc returns a falsy value (e.g., false or undefined), the original address is displayed.

    Example without mapping:

    null  ➡️  EQBGhqLAZseEqRXz4ByFPTGV7SVMlI4hrbs-Sps_Xzx01x8G
          ➡️  0.05 💎 EQC2VluVfpj2FoHNMAiDMpcMzwvjLZxxTG8ecq477RE3NvVt
    

    Example with mapping:

    null  ➡️  Torch's Wallet
          ➡️  0.05 💎 Alan's Wallet
    
  2. Operation Code Mapping in printTransactionFees:

    • Added an optional OpMapFunc to map operation codes (op) to human-readable formats in the transaction fees log.
    • When mapFunc is provided, op values are transformed using this function. If not, they are displayed in hexadecimal by default.

    Example without mapping:

    ┌─────────┬─────────────┬────────────────┬────────────────┬────────────────┬────────────────┬───────────────┬────────────┬────────────────┬──────────┬────────────┐
    │ (index) │ op          │ valueIn        │ valueOut       │ totalFees      │ inForwardFee   │ outForwardFee │ outActions │ computeFee     │ exitCode │ actionCode │
    ├─────────┼─────────────┼────────────────┼────────────────┼────────────────┼────────────────┼───────────────┼────────────┼────────────────┼──────────┼────────────┤
    │ 0       │ 'N/A'       │ 'N/A'          │ '1000 TON'     │ '0.004007 TON' │ 'N/A'          │ '0.001 TON'   │ 1          │ '0.001937 TON' │ 0        │ 0          │
    │ 1       │ '0x45ab564' │ '1000 TON'     │ '998.8485 TON' │ '1.051473 TON' │ '0.000667 TON' │ '0.255 TON'   │ 255        │ '0.966474 TON' │ 0        │ 0          │
    

    Example with mapping:

    ┌─────────┬─────────────┬────────────────┬────────────────┬────────────────┬────────────────┬───────────────┬────────────┬────────────────┬──────────┬────────────┐
    │ (index) │ op          │ valueIn        │ valueOut       │ totalFees      │ inForwardFee   │ outForwardFee │ outActions │ computeFee     │ exitCode │ actionCode │
    ├─────────┼─────────────┼────────────────┼────────────────┼────────────────┼────────────────┼───────────────┼────────────┼────────────────┼──────────┼────────────┤
    │ 0       │ 'CustomOp1' │ 'N/A'          │ '1000 TON'     │ '0.004007 TON' │ 'N/A'          │ '0.001 TON'   │ 1          │ '0.001937 TON' │ 0        │ 0          │
    │ 1       │ 'CustomOp2' │ '1000 TON'     │ '998.8485 TON' │ '1.051473 TON' │ '0.000667 TON' │ '0.255 TON'   │ 255        │ '0.966474 TON' │ 0        │ 0          │
    

Motivation:

This change provides more flexibility in transaction logging, making the logs easier to read and understand for developers who work with mapped addresses or operation codes. It also ensures that if no mapping is provided, the default logging behavior remains intact.

Backwards Compatibility:

  • The existing functionality is preserved. If no mapping functions (mapFunc) are passed, the original src, dest, and op values are logged as before.

@alan890104
Copy link
Author

@krigga Thanks for your advice! I have made the suggested modifications, ensuring that mapping is now handled using a function for better scalability and flexibility.

Copy link

@hossen71 hossen71 left a comment

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

@hossen71
Copy link

hossen71 commented Dec 5, 2024

hi

@alan890104
Copy link
Author

Hi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants