simulate: fix signers

[joe-p]

Summary

This is a new feature in simulate that will close #5914. Auth address correction is done is two places:

1. In simulateWithTracer the txn fields will be analyzed and auth addrs for txns will be updated past on previous rekeyTo fields
2. In AfterProgram the auth addr field for all txns up to the next app call will be corrected

This allows one to simulate with empty signatures without knowing the proper auth addresses when forming the group.

Test Plan

There is basic test coverage right now, but more tests ensuring all code paths are covered need to be written

TODO

We need to decide when/how this is enabled. To me it makes the most sense to have this as a new query parameter for the simulate endpoint

[joe-p]

@jasonpaulos let me know what you think about this general approach. If it looks good, I can work on the endpoint and more tests

[joe-p]

@jasonpaulos this comment in the test you initially wrote got me thinking... Should we expect the transaction group to be modified in the response? Is there any precedent for this?

I feel like there is value in seeing the txn group as it was given and also seeing what was modified. Perhaps an additional field that indicates a changed signer per txn, but the actual transaction object remains unchanged?

[joe-p]

Also I'm switching from the terminology "fix auth addr" to "fix signer" to avoid confusion between the auth addr for a given account in the ledger and the signer for the transaction, making it clear the ledger remains unchanged and its just txns that are getting modified.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 72.88136% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 55.95%. Comparing base (135a1b4) to head (156c3e9). Report is 58 commits behind head on master.

Files	Patch %	Lines
ledger/simulation/tracer.go	57.14%	4 Missing and 5 partials :warning:
ledger/simulation/simulator.go	81.08%	5 Missing and 2 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5942      +/-   ##
==========================================
- Coverage   55.96%   55.95%   -0.01%     
==========================================
  Files         478      478              
  Lines       67601    67648      +47     
==========================================
+ Hits        37835    37855      +20     
- Misses      27204    27220      +16     
- Partials     2562     2573      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[jasonpaulos]

@jasonpaulos this comment in the test you initially wrote got me thinking... Should we expect the transaction group to be modified in the response? Is there any precedent for this?

I feel like there is value in seeing the txn group as it was given and also seeing what was modified. Perhaps an additional field that indicates a changed signer per txn, but the actual transaction object remains unchanged?

I agree that it would be preferable to not modify the transactions as given, and instead provide an additional response field with the overridden signer. The only problem is that the response structure may make it difficult to associate this additional information with inner transactions.

[joe-p]

The only problem is that the response structure may make it difficult to associate this additional information with inner transactions.

Unless you are thinking of functionality that I am not, we're only modifying the signers on the outer transactions, so we only need to add additional information to the outers.

[jasonpaulos]

Unless you are thinking of functionality that I am not, we're only modifying the signers on the outer transactions, so we only need to add additional information to the outers.

Good point, this plan should have no problems then