BM-601: Simplify callback logic by removing (somewhat weak) call-once property

[nategraf]

This PR is a change I am proposing, but do not have consensus on. Opening it to discuss.

With #335, we added support for callbacks on fulfillment and we aimed to have the callback trigger at most once per request. Since that PR, I realized two issues:

1. As implemented, there is a potential re-entrancy attack on this property in that the called contract can call one of the fulfill methods and have the callback trigger again for the same request (ID). This is because we use the fulfilled flag to indicate whether the callback has run, but that flag is not set until after the callback is complete.
2. Even if a given request ID can only be used to trigger a callback once, any user can create an arbitrary number of unique requests that are fulfilled by the same proof. Using a single receipt, they can fulfill an arbitrary number of requests and trigger an identical callback for each. This is indistinguishable on the called contract from multiple calls associated with the same request.

My opinion is that the call-once property does not buy us much. If there is an action which modifies state, the called application will have its own notion of what to do when the callback arrives, and ensuring it behaves reasonably if the same call is repeated is a common requirement for a wide range of applications.

As an advantage to dropping this property as a goal, benchmarks show it would save ~800 gas per fulfillment.

[willpote]

As implemented, there is a potential re-entrancy attack on this property in that the called contract can call one of the fulfill methods and have the callback trigger again for the same request (ID). This is because we use the fulfilled flag to indicate whether the callback has run, but that flag is not set until after the callback is complete.

Good catch, this was actually why I originally implemented the callback to be the last action. I totally forgot when reviewing the refactor

Even if a given request ID can only be used to trigger a callback once, any user can create an arbitrary number of unique requests that are fulfilled by the same proof. Using a single receipt, they can fulfill an arbitrary number of requests and trigger an identical callback for each. This is indistinguishable on the called contract from multiple calls associated with the same request.

Great point. Something we should definitely capture in our docs for this feature