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

Implement logging #183

Merged
merged 2 commits into from
Nov 8, 2024
Merged

Implement logging #183

merged 2 commits into from
Nov 8, 2024

Conversation

virgil-serbanuta
Copy link
Member

No description provided.

Copy link

@Robertorosmaninho Robertorosmaninho left a comment

Choose a reason for hiding this comment

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

LGTM.
Executed and tested.

 'logs': [AttributeDict({'address': '0x9D11e3Af4cAb7A54394b93Fb12843EB7FB3Da3D4', 'topics': [HexBytes('0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef'), HexBytes('0x0000000000000000000000008e156a2d91c51181cd3bb6e5e45db7221ed66312'), HexBytes('0x0000000000000000000000001111111111111111111111111111111111111111')], 'data': HexBytes('0x000000000000000000000000000000000000000000000000000000000000002a'), 'blockNumber': 1019, 'transactionHash': HexBytes('0xab0d97a6eb79e91a187ce3a7ca83d88ec97d076a0d823a7d5b1e507b65a83aa5'), 'transactionIndex': 0, 'blockHash': HexBytes('0xb7d40cbe71a9ceb7a23b7bf485a7f256f61322f57a02d99c7e6d99d4d1cf8b73'), 'logIndex': 0, 'removed': False})]

Copy link

@mariaKt mariaKt left a comment

Choose a reason for hiding this comment

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

LGTM. I have a couple of questions, mainly for my understanding.

)
=> #ulmPreprocessEvent
( Method
, appendParamToBytes(data, last(Param, Params))
Copy link

Choose a reason for hiding this comment

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

Do I understand correctly that this is handling specifically the case of an event with only indexed parameters, and one last non indexed parameter? It is fine since this is the case for the event that we want to handle, just checking.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're right. I don't understand Ethereum event logging well enough to know exactly what I did here (logging was mostly ignored until yesterday (or the day before?) when it suddenly became high priority, and I didn't have time to learn enough about it). However, I think that it's doing the same thing as the SIMPLE example, and I think that it would be easy to change it to something better in the future. I'll add a TODO.

@@ -8,16 +8,19 @@ list_mock GetAccountStorageHook ( 1154948450467236006739439905978168116697077396
list_mock SetAccountStorageHook ( 115494845046723600673943990597816811669707739681772931244236289759170204726560 , 9900 ) ulmNoResult();
list_mock GetAccountStorageHook ( 17171626450567201640701347902808840427582371480602455275836469707331258301780 ) ulmIntResult(0, u256);
list_mock SetAccountStorageHook ( 17171626450567201640701347902808840427582371480602455275836469707331258301780 , 100 ) ulmNoResult();
list_mock Log3Hook ( 100389287136786176327247604509743168900146139575972864366142685224231313322991 , 1010101 , 2020202 , b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00d" ) ulmNoResult();
Copy link

Choose a reason for hiding this comment

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

Is this the expected topic0, topic1, topic2, and bytes corresponding to an event in erc20? If so, which one, and how can I generally figure out which one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, these are the topics, in order, and the bytes. These mocks (storage + logs) were developed in a hurry, when we already not fully confident that we can finish until the demo, so we cut corners. They should be implemented in a similar way to the call mocks (see examples in the same file, below), and call mocks themselves could be implemented better (e.g. we could put the entire function signature in a single string instead of building it piece by piece).

If there is interest in the Rust semantics after the demo, this is one of the things that should be fixed.

Copy link
Member

@yanliu18 yanliu18 left a comment

Choose a reason for hiding this comment

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

LGTM.

@virgil-serbanuta virgil-serbanuta merged commit ff3d6ef into main Nov 8, 2024
3 checks passed
@virgil-serbanuta virgil-serbanuta deleted the logging branch November 8, 2024 15:25
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