ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.45k stars 5.8k forks source link

[isoltest] Handle events in isoltest semantics tests. #6902

Closed ekpyron closed 3 years ago

ekpyron commented 5 years ago

At some point we should be able to expect events emitted during a transaction/call in isoltest. We'll first need to decide what syntax to use for that, though. Suggestion:

contract C {
    event TestEvent(uint256);
    function f() public {
        emit TestEvent(42);
    }
}
// ----
// f() ->
// LOG[0]: TestEvent(uint256), 42

Maybe we should also have some way to check the logged ether amount of the transaction.

axic commented 4 years ago

Moved from #10427:

Some tests depend on checking numLogTopics(logIndex) -> uint, logAddress(logIndex) -> address, logData(logIndex) -> bytes, logTopic(logIndex, topic) -> bytes, such as abi_encodePackedV2_structs.

I have no syntax suggestion, but this may be a blocker for getting rid of SolidityEndToEndTest.

ekpyron commented 3 years ago

Some tests depend on checking numLogTopics(logIndex) -> uint, logAddress(logIndex) -> address, logData(logIndex) -> bytes, logTopic(logIndex, topic) -> bytes, such as abi_encodePackedV2_structs. I have no syntax suggestion, but this may be a blocker for getting rid of SolidityEndToEndTest.

All of this would be covered by just checking the entire log precisely, though, wouldn't it? I.e. in abi_encodePackedV2_structs the following would be fine:

// testStorage() ->
// LOG[0]: E(string[]) -> ... /* encoding */
// testMemory() ->
// LOG[0]: E(string[]) -> ... /* same encoding */

Especially, if we make events not occurring in the expectations an error (which we definitely should).

axic commented 3 years ago

Yes, all are reading parts of the same logIndex supplied. This was a direct translation what is in the tests currently, not a definitive suggestion. However since logs can be large, perhaps it is easier to read if it is split up.

ekpyron commented 3 years ago

Ok, logAddress in some of the other tests is still a thing. But if we don't come up with something better we can just make the address part of the expectation in general (the addresses in the tests are deterministic)...

aarlt commented 3 years ago

I was experimenting a bit with the syntax and how we could implement this. I was thinking about introducing "builtin" functions into isoltest. I thought it would make sense to provide the same "low-level" functions - numLogs, numLogTopics, logTopic, logAddress and logData - as used in SolidityEndToEndTest.cpp.

I think @ekpyron syntax proposal is nice but maybe it is too high-level. If I understood that syntax correctly, we will not be able to check explicitly for indexed & anonymous events. I could imagine that it may make sense to have such "higher-level" function. Such function may simplify the event usage of newly written semantic tests. I think we should start with the "low-level" functions first, so that we are able to easily translate the event based SolidityEndToEndTest tests to semantic tests.

I think the syntax for the "low-level" functions could look like this:

// logs.numLogs() -> 1
// logs.numLogTopics(uint256): 0 -> 2
// logs.logTopic(uint256,uint256): 0, 0 -> 0x123...
// logs.logAddress(uint256): 0 -> 0x234..
// logs.logData(uint256): 1 -> 0x345..

I have in mind to create a simple framework that allows to define builtin functions. These builtin functions will be placed in a "module" namespace, where each module can provide different functions. A call to a builtin will always be defined like <module>.<builtin-function>(<parameter-types) and follow the syntax of normal functions calls defined by the semantic tests. The difference is, that the builtin functions will contain a dot to specify the module and a function within that module. So for now, a normal call will never contain any dot, where a call to a builtin function will always contain a dot.

I think this will make it more easy to extend the semantic tests with additional functions.

See https://github.com/ethereum/solidity/issues/10474 and https://github.com/ethereum/solidity/issues/10426.

chriseth commented 3 years ago

I think for logs, this is much too detailed, and we can keep the 2-3 tests that cannot use the event interface in EndToEndTests, but it could be useful for balance.

aarlt commented 3 years ago

I think for logs, this is much too detailed, and we can keep the 2-3 tests that cannot use the event interface in EndToEndTests, but it could be useful for balance.

Yes these "low-level" builtins are too detailed, but I added a new builtin logs.expectEvent(uint256,string) that is more "high-level" . It is quite similar to @ekpyron proposal. See comment.