fix(promise): Disallow invalid batch_then Promise syntax

[austinabell]

#263: The drops were inconsistent because when a promise was queued using then twice, the first would be dropped because it would be replaced with the other one, causing it to be queued through the blockchain interface before the other drop occurs.

Reading the code for this, it was quite confusing why the drops are currently called in reverse order.

in the case of p1.then(p2).then(p3), the drops are called in p3 -> p2 -> p1 and also the queues are also in that order, which means that the batch_then is called for p3 before p2 then the batch_create is called with p1. This is how they were queued before the change, so I avoided making a breaking change until I understand why everything is queued like this (do the actions get pulled off like a stack?).

This change just makes it so that the combinations referred to in the issue now call all things through the blockchain interface consistently.

Closes #263

[MaksymZavershynskyi]

Reading the code for this, it was quite confusing why the drops are currently called in reverse order.

As explained here, promises implement RAII pattern, because of the RAII execution of drops is happening in the reverse order from the initialization, because this is how stack with local variables works -- it deallocates variables in reverse order from their initialization.

It is however irrelevant in what order contract calls are scheduled, since there is no notion of order for them. Dependencies between receipts form a DAG and not necessarily a sequence.

Instead of making the syntax mentioned in the issue work, we need to explicitly prohibit it, because it is hard to reason about the expected result from it.

[austinabell]

As explained here, promises implement RAII pattern, because of the RAII execution of drops is happening in the reverse order from the initialization, because this is how stack with local variables works -- it deallocates variables in reverse order from their initialization.

I get this, I guess I was confused as to how the promises are queued through the blockchain interface.

Instead of making the syntax mentioned in the issue work, we need to explicitly prohibit it, because it is hard to reason about the expected result from it.

Sounds good, I'll do that now, although I should definitely dig deeper into the internals of promise scheduling here. Would be ideal if this check could be done at compile-time, but I can just panic if the Promise is already to be queued.

[evgenykuzyakov]

Maybe we should replace this panic and the one below with env::panic

[austinabell]

Just to update here, since the direction of this functionality is not in consensus, I will wait for the intentions and ideal API to be discussed in #263 and add information as I learn the internals better

Fixed changes and Max is OOO

[ChaoticTempest]

Makes sense to disable this pattern for now

[itegulov]

I assume we need final approval from @nearmax on this?

[austinabell]

I assume we need final approval from nearmax on this?

I'll wait until after the weekend to merge but he's OOO and we went with the option he was proposing so probably not necessary.