starknet-io / starknet.js

JavaScript library for StarkNet
https://www.starknetjs.com
MIT License
1.22k stars 747 forks source link

Bug parsing events when nested events involved #1134

Closed NikitaMishin closed 4 months ago

NikitaMishin commented 5 months ago

Location: https://github.com/starknet-io/starknet.js/blob/66a5c0341eccfef0dcdf1312c15627b7d4f6b675/src/utils/events/index.ts#L52C21-L52C28

Encounter issue with parsing events emitted by components:

Sample of code (just take any events that emitted in nested components):

    const eventsC = provider.getEvents(....)
    const abiEvents = STARKNETJS.events.getAbiEvents(abi);
    const abiStructs =  STARKNETJS.CallData.getAbiStruct(abi);
    const abiEnums = STARKNETJS.CallData.getAbiEnum(abi);
    let parsedEvents = STARKNETJS.events.parseEvents(eventsC.events, abiEvents, abiStructs, abiEnums)
    console.log(parsedEvents)

produces 0 parsed events

Reason: AbiEvents not handle correctly case for nested events when they defined by extra key that associated with component. In my particular case second key was the correct one to do lookup to abiEvents Potential solution:

let abiEvent: EventAbi | undefined;

// Loop through all keys and find the first matching event
for (const key of recEvent.keys) {
    abiEvent = abiEvents[key];
    if (abiEvent) {
        break; // Exit the loop once a match is found
    }
}

iterate over all keys/or first N instead of just first one. I presume there should be no collisions

MartinXV111 commented 5 months ago

can i hop on this? @NikitaMishin

martinvibes commented 5 months ago

hi @NikitaMishin i will love to hop on this one

NikitaMishin commented 5 months ago

also if several components have same name for event getAbiEvents would just shallow rewrite it, not taking into account component hash

Timilehin-bello commented 5 months ago

@NikitaMishin please can you kindly assign this task to me. I'll start working on it and revert

PhilippeR26 commented 5 months ago

Hello @NikitaMishin , I would like to repeat completely your problem. I need the network used, the parameters in getEvents, the address of the contract of these events.

NikitaMishin commented 5 months ago

Hello @NikitaMishin , I would like to repeat completely your problem. I need the network used, the parameters in getEvents, the address of the contract of these events.

Hi! network is SEPOLIA, range for block search: from block 69198, to block 69198 contract 0x07981ea76ca241100a3e1cd4083a15a73a068b6d6a946d36042cbfc9b531baa2 (in particular for example this https://sepolia.voyager.online/event/69198_3_3 event)

const result = await this.provider.getEvents({
        address: this.exchangeAddress,
        from_block: { block_number: fromBlock },
        to_block: { block_number: toBlock },
        keys: [
          [], /// first key refers to component key
          [num.toHex(hash.starknetKeccak('Trade'))],
          ...restKeys,
        ],
        chunk_size: chunkSize,
        continuation_token: continuationToken,
      });
      ....
      const { abi: abi } = await provider.getClassAt(contractAddress);

      this.abiEvents = events.getAbiEvents(abi);
      this.abiStructs = CallData.getAbiStruct(abi);
      this.abiEnums = CallData.getAbiEnum(abi);

      ....

      const parsedEvents = events.parseEvents(
        result.events,
        abiEvents ?? this.abiEvents,
        this.abiStructs,
        this.abiEnums,
      );
PhilippeR26 commented 5 months ago

What's the content of restKeys?

NikitaMishin commented 5 months ago

What's the content of restKeys?

just to specify indexed keys like maker and taker for this particular event. u can omit specifying them and put []

PhilippeR26 commented 5 months ago

The first key is 0x22ea134d4126804c60797e633195f8c9aa5fd6d1567e299f4961d0e96f373ee. I suppose that it's a starknetKeccak of some text. Do you know which text is it?

PhilippeR26 commented 5 months ago

Seems to be the hash of BalancerEvent

PhilippeR26 commented 5 months ago

OK, I have now a good understanding of this problem. I will use a nested findIndex / includes to find the right key.

NikitaMishin commented 5 months ago

also if several components have same name for event getAbiEvents would just shallow rewrite it, not taking into account component hash

@PhilippeR26, this issue is related. I assume getAbiEvents does not take the component hash into account. If two events have the same base name, it will overwrite one with the other. For example, I defined similar events with the same name "Deposit" in two components, and it resulted in an incorrect ABI, only recognizing one of the events instead of both.

For this maybe would be sufficient to have getAbiComponentEvents to avoid breaking change for people who uses getAbiEvents and just mention about this somewhere in the docs?

PhilippeR26 commented 5 months ago

I think it's a good guideline to not have 2 times the same name in an abi. Otherwise you are going straight to problems. Is it possible to have events nested 2 times (use of a Cairo component that uses a sub-component that emits events)?

NikitaMishin commented 5 months ago

I think it's a good guideline to not have 2 times the same name in an abi. Otherwise you are going straight to problems. Is it possible to have events nested 2 times (use of a Cairo component that uses a sub-component that emits events)?

Yes, possible (if i correctly understood your q): https://github.com/LayerAkira/kurosawa_akira/blob/53e50c3e776fe352dcdc8160d1666d7a3628690a/src/DepositComponent.cairo#L23 https://github.com/LayerAkira/kurosawa_akira/blob/53e50c3e776fe352dcdc8160d1666d7a3628690a/src/RouterComponent.cairo#L88

PhilippeR26 commented 5 months ago

My question was : is it possible to use in Cairo a component in a component?

NikitaMishin commented 5 months ago

My question was : is it possible to use in Cairo a component in a component?

yes in a sense that you can interact with other components in component

PhilippeR26 commented 5 months ago

🥴 I don't know how to ask my question in an understandable way ... Let's try an other way : At the end, is it possible to have an event with a key array including more than 2 hashes to define the event name?

NikitaMishin commented 5 months ago

🥴 I don't know how to ask my question in an understandable way ... Let's try an other way : At the end, is it possible to have an event with a key array including more than 2 hashes to define the event name?

oh, this one i dont know tbh i think not possible (but need some cairo dev let me ask some guys)

PhilippeR26 commented 5 months ago

Hello, I coded something that is recursive and can handle several levels of components (even if it's maybe not possible today). Could you try with this line in your package.json? :

"starknet": "philippeR26/starknet.js#c9fab45b78222154247d8078bb60af2dd30a9611",

Now, the result contains the full name of the event (was just the last word), to differentiate the events with the same name. Waiting your response.

NikitaMishin commented 5 months ago

Hello, I coded something that is recursive and can handle several levels of components (even if it's maybe not possible today). Could you try with this line in your package.json? :

"starknet": "philippeR26/starknet.js#c9fab45b78222154247d8078bb60af2dd30a9611",

Now, the result contains the full name of the event (was just the last word), to differentiate the events with the same name. Waiting your response.

ok, ill try in a tad later today, thanks!

PhilippeR26 commented 5 months ago

For the test suite of Starknet.js, could you provide me a link in Sepolia for an example these events :

NikitaMishin commented 5 months ago

Hello, I coded something that is recursive and can handle several levels of components (even if it's maybe not possible today). Could you try with this line in your package.json? :

"starknet": "philippeR26/starknet.js#c9fab45b78222154247d8078bb60af2dd30a9611",

Now, the result contains the full name of the event (was just the last word), to differentiate the events with the same name. Waiting your response.

yeap works

PhilippeR26 commented 5 months ago

For the test suite of Starknet.js, could you provide me a link in Sepolia for an example these events :

* kurosawa_akira::DepositComponent::deposit_component::Deposit

* kurosawa_akira::RouterComponent::router_component::Deposit

Do you have also in your code an example of a nested event tagged #[flat]?

NikitaMishin commented 5 months ago

For the test suite of Starknet.js, could you provide me a link in Sepolia for an example these events :

* kurosawa_akira::DepositComponent::deposit_component::Deposit

* kurosawa_akira::RouterComponent::router_component::Deposit

Do you have also in your code an example of a nested event tagged #[flat]? i didn't use #flat tbh seems like still not prod ready given some old discussions in cairo group. deposit event: https://sepolia.voyager.online/event/70352_34_3 router deposit event: https://sepolia.voyager.online/event/70352_29_3

they are on same block

PhilippeR26 commented 5 months ago

Found some [#flat] in OpenZeppelin contracts. I have found a corresponding event. So, I finalize and clean my code. then, I will propose quickly a PR.

NikitaMishin commented 5 months ago

Found some [#flat] in OpenZeppelin contracts. I have found a corresponding event. So, I finalize and clean my code. then, I will propose quickly a PR.

top! very quick, thanks!