413 improve private key management in bls provider and signer

[JohnGuilding]

What is this PR doing?

This PR removes all references to the signer in the BlsProvider. This reduces the tight coupling between the two classes, better represents a JsonRpcProvider, and we can remove code like:

if (!this.signer) {
      throw new Error("Call provider.getSigner first");
}

Other changes:

1. Moved the constructTransactionResponse and constructTransactionBatchResponse methods to the provider. This consolidates the logic in one place.
2. Added _validateTransaction and _validateTransactionBatch methods in the signer to split validation logic. This reduces the complexity of the send transaction methods and makes testing easier.
3. Moved some tests from integration tests to ./clients as they don't require a running network to pass.
4. Removed some regular JsonRpcProvider and JsonRpcSigner tests at the bottom of the BlsProvider.test.ts and BlsSigner.test.ts files.
5. Cleans up provider and signer code and test code

How can these changes be manually tested?

run integration tests and clients tests

Does this PR resolve or contribute to any issues?

#413
Also fixes #427. Turns out I wasn't funding the new signer in the test.

Checklist

• I have manually tested these changes
• Post a link to the PR in the group chat

Guidelines

• If your PR is not ready, mark it as a draft
• The resolve conversation button is for reviewers, not authors
  • (But add a 'done' comment or similar)

[JohnGuilding]

Open to suggestions on being able to estimate fees in the provider without having reference to a signer. This was my hacky solution. It does result in slightly different gas estimates each time, but still close enough to send the transaction successfully.

See this comment for examples of differences in tests without mocking a consistent private key value.

Not a fan of creating a throwaway BlsWalletWrapper, but feels slightly better than having a signer tightly coupled to a provider.

[jacque006]

I think for now this is fine. I would document why your are doing this in code.

We should have an estimation endpoint/method for v2 that does not require a signed bundle.

[JohnGuilding]

Added a comment and tracked via a follow up issue for V2 #560

[JohnGuilding]

Example of differences observed without sinon.fake:

Test	Expected Fee Estimate	Fee Estimate
1	0.000079145258744232	0.000077749364772554
2	0.00006806648707833	0.000069299213638008
3	0.000060669559559918	0.000060666067046826

[JohnGuilding]

It is possible in the provider to get a bundle with multiple public keys and operations, this logic ensures all of the transactions in transactions: Array<ethers.providers.TransactionResponse> = [] are linked to the correct nonce and from address.

e.g. if I just mapped across all of the actions like I was before, then there would be no way of telling which public key was linked to which group of actions.

[JohnGuilding]

This is a subtle, but important change, it ensures we don't require the following logic in many places and have to enforce adding from to transaction requests:

if (!resolvedTransaction.from) {
   throw new TypeError("Transaction.from should be defined");
}

This is because the provider estimateGas method now requires from to calculate the nonce, and the transaction that this method is validating will get pass to that method later in the send transaction methods. See changes to estimateGas and this comment for more context

[JohnGuilding]

from needed to be added manually here as calling estimateGas directly. The _validateTransaction method on the signer does this work for us in cases where the transaction comes from the signer.