recoverNSignatures() should return all signatures instead of number of signatures specified by requiredSignatures

[jimmychu0807]

Have you ensured that all of these are up to date?

• This repo
• Any dependencies (according to the package.json)

What command(s) is the bug in?

No response

Operating System

None

Describe the bug

recoverNSignatures() function currently only returns requiredSignatures number of signatures.

There could be a case that there are three signatures that are [valid, invalid, valid] with requiredSignatures = 2. In this case, the caller of this function will never retrieve the last valid signature.

I'm looking at this function in the context of OwnableValidator.sol recoverNSignatures(). If it requires the caller to know the total num of signatures beforehand in order to retrieve all sign. , then the param used in OwnableValidator.sol recoverNSignatures() should not be _threshold but the total number of signatures computed by the caller.

So seems need to update either the caller side or how recoverNSignatures() works.

@kopy-kat see if this suggestion makes sense to you. Thanks.

[jimmychu0807]

I am okay to help make a PR by returning all signatures from recoverNSignatures(), and adding a test case in OwnableValidator for this case, if you also agree with this direction.

Then recoverNSignatures() function won't need the requiredSignatures parameter and use signaturesLength / 65 to determine the num. of signatures.