ewasm / design

Ewasm Design Overview and Specification
Apache License 2.0
1.02k stars 125 forks source link

Strict argument handling of log() #129

Open axic opened 6 years ago

axic commented 6 years ago

log has a numberOfTopics argument and four topic offsets. Depending on the value of numberOfTopics (0 to 4) the offsets are used or not.

Should the interface ensure that any unused offsets must be strictly set to 0?

lrettig commented 6 years ago

I'm all for explicit error condition checking so I say yes.

axic commented 6 years ago

@chfast @holiman can you please raise your voices here?

holiman commented 6 years ago

Not sure I understand the full context. So this is during runtime, when a contract calls the log(numberoftopics, o1,o2,o3,o4) or what? Could you examplify ?

axic commented 6 years ago

Yes, during runtime. Currently numberOfTopics controls how many of the topic counters are used. If say numberOfTopics is 2, then topic3 and topic4 are ignored entirely. The question is: should even in that case ewasm enforce that they are set to a known value (such as 0) or not?

holiman commented 6 years ago

So, let me rephrase... If there is an error here, and topics3 is non-zero when it should be -- who have made the error? The compiler? The contract developer?

axic commented 6 years ago

What do you mean by error here? This function has no error reporting.

It can fail if the topic pointers are out of bounds or the the number of topics field is set higher than the validly supplied pointers.

Whether this is the fault of the compiler or the developer depends on the context. We expect that high level DSLs (such as Solidity) would handle this in the compiler and as a result would be a compiler bug. In less tight coupling (such as in system contracts) it is the developers fault.

jakelang commented 5 years ago

I think it doesn't make sense to enforce that unused topic pointers are zero. The implementation of log should not be trying to dereference those anyway.

On the other hand, in the case that numberOfTopics is set higher than the number of valid topic pointers passed, it is a dev error and should clearly specify what occurs in the case that log tries to read topic data from an OOB pointer.

axic commented 5 years ago

One benefit of requiring them be zero is it could aid static analysers.

An alternative solution is to require all topics to be laid out in linear memory and in that case there would be a single pointer + the numberOfTopics.

Having multiple pointers gives flexibility in arranging the data from the callers side.