Transaction with multiple actions can create identical requests

[mattlockyer]

Description

If a transaction is executed with multiple actions to call sign with identical payloads, all of these sign calls will pass the check at line:

mpc/chain-signatures/contract/src/lib.rs

Line 164 in ecc6fd3

if !self.request_already_exists(&request) {

Because the contract only adds the request after an async call to sign_helper.

If multiple actions are used, then multiple calls to sign_helper will be fired, resulting in multiple calls to env::promise_yield_create which may go on to create further issues.

See transaction here:
https://testnet.nearblocks.io/txns/FWB6Wq8tYNLFgB3zFnHZ5rmunfEHuVKscowakeenLAMq#execution

[ppca]

This check problem can be fixed by marking such request present before going into sign_helper().
Before I fix it, I want to know do you hope to be able to send multiple sign() with exactly same payload in one batch transaction? What is the use case?
@mattlockyer

[mattlockyer]

No. I do not wish to send multiple sign requests with the same payload.

I was just flagging this because it may have caused further issues later in the runtime of the contract.

[volovyks]

@ppca our testnet contract is broken and I think this issue is the reason.
When submitting a transaction with 3 sign actions we increase our counter by 3, but then clear_state_on_finish fails with "out_of_gas" error. As a result - the counter never decreases and the price is maxed out. All the following requests are failing because of this.

[ppca]

this should be fixed by the PR too.