Ad-hoc Concurrency Helpers

galfour · January 22, 2021, 4:05pm

Hi,

I am looking into small fixes that could improve the situation with regard to concurrency, as described in this thread, and as implemented in this MR.

The various small fixes that I have in mind are the following:

A self-lock preventing re-entrency by default. Re-entrency is a common vector of attacks, and except when it is actively needed, it might be good to disallow it by default.
Add a flag to be called as a DFS contract. All operations calling a contract with this flag are treated as DFS operations.
Add a MAKE_STERILE instruction, that prevents an internal operation from spawning children operations. (After more thinking about it, it is less useful than envisioned, but possibly, other people have other ideas.)

From @murbard’s comment:

Preventing MAKE_DFS in contracts called by legacy contracts.
Explicitly adding an opcode ALLOW_DFS that lets children contracts use MAKE_DFS.

@cesar @Martin @EGuenz

EGuenz · January 22, 2021, 6:07pm

Regarding @murbard’s suggestions, I think implementing the MAKE_DFS instruction to mean labelling the emitted operation contextual (as discussed in Concurrency, BFS vs DFS (and a proposal)) instead of dfs clears up the worry these suggestions are addressing.

See my comment on the tzip discussion: https://gitlab.com/tzip/tzip/-/merge_requests/111#note_492008977

murbard · January 22, 2021, 6:20pm

It’s not clear to me how contextual operations help. The point of DFS is wanting the guarantee that after something executes you’re going to be called right back and nothing else will happen in between.

tom · January 26, 2021, 5:37pm

I am not following the discussion well, but, a minor note:

It seems this may cause liveness issues which could be easily missed.

A hypothetical story: I am testing two contracts. One is exactly like my old mainnet contract. It will call the other contract, a new one which uses MAKE_DFS. Everything works fine in mockup/sandbox. I originate the new contract on mainnet …and it doesn’t work because the old one is “legacy.”

Liveness issues would seem particularly sad if the legacy contract didn’t actually depend on BFS anyway…

m-kus · January 27, 2021, 8:56am

Is it really necessary to do switching between BFS/DFS in runtime? Wouldn’t it be better to specify that at the origination (using a flag)? Then it would be more predictable, plus no need to introduce instructions that alter context (so far all context-dependent instructions were read-only).