forta-network / forta-bot-sdk-v2

1 stars 0 forks source link

create_transaction_event Does Not Properly Parse Dictionary (Python) #5

Open benefacto opened 1 month ago

benefacto commented 1 month ago

I encountered an issue with the create_transaction_event function in the Python SDK. It appears that this function does not properly parse this example dictionary and requires constructing the TransactionEvent directly. This issue causes unexpected errors during test executions.

Steps to Reproduce

Use the following example dictionary for creating a transaction event:

from forta_agent import create_transaction_event
from datetime import datetime

event_dict = {
    "transaction": {
        "hash": "0",
        "from": "0x0000000000000000000000000000000000000000",
        "nonce": 9,
    },
    "block": {
        "number": 0,
        "timestamp": datetime.now().timestamp(),
    },
    "receipt": {"logs": []},
    "chain_id": 1,
}

tx_event = create_transaction_event(event_dict)

Expected Behavior

The create_transaction_event function should parse the example dictionary correctly and create a TransactionEvent object without requiring additional manual steps.

Actual Behavior

The function raises a TypeError indicating that the block and chain_id arguments are missing, forcing the user to construct the TransactionEvent directly.

Workaround

Currently, I have to construct the TransactionEvent directly to avoid the error:

from forta_agent import TransactionEvent

event_dict = {
    "transaction": {
        "hash": "0",
        "from": "0x0000000000000000000000000000000000000000",
        "nonce": 9,
    },
    "block": {
        "number": 0,
        "timestamp": datetime.now().timestamp(),
    },
    "receipt": {"logs": []},
    "chain_id": 1,
}

tx_event = TransactionEvent(event_dict)

Environment

Additional Information

This issue was encountered while running test cases using pytest while migrating a Forta starter kit bot to V2. The error messages indicated that the block and chain_id arguments were missing, even though they were included in the provided dictionary.

haseebrabbani commented 1 month ago

hey @benefacto πŸ™‚ appreciate the report. the function signature for create_transaction_event has actually changed in V2. specifically it accepts multiple arguments:

def create_transaction_event(transaction: dict | Transaction, block: dict | Block, chain_id: int, traces: list[Trace] = [], logs: list[Log] = [])

if you want to just pass in a dict, you can use TransactionEvent directly. but there is some processing done in create_transaction_event that would be missing. this may be fine for your tests, but just a heads up

benefacto commented 1 month ago

hey @benefacto πŸ™‚ appreciate the report. the function signature for create_transaction_event has actually changed in V2. specifically it accepts multiple arguments:

def create_transaction_event(transaction: dict | Transaction, block: dict | Block, chain_id: int, traces: list[Trace] = [], logs: list[Log] = [])

if you want to just pass in a dict, you can use TransactionEvent directly. but there is some processing done in create_transaction_event that would be missing. this may be fine for your tests, but just a heads up

Thanks for clarifying the function signature, @haseebrabbani . To me, this signature violates the principle of least astonishment, since when passing in a dictionary including the block and chain_id alongside the transaction details (as was previously done in V1), I would not expect to have to pass it in a second time to fulfill the other required parameters.

If the goal is to maintain a high degree of flexibility for the arguments passed, it would be more intuitive to accept a single object with a number of optional properties. This approach would streamline the function call and reduce the potential for errors. Since the V1 version constructs the object directly from a dictionary, adding additional properties to a single parameter object could maintain backwards compatibility while ensuring that developers are making the necessary upgrades for V2.