SessionKit usage of SigningRequest vs ResolvedSigningRequest during transact

[aaroncox]

While working on wharfkit/session#53 in an attempt to make it possible to manually create a Session and sign a transaction offline (e.g. no API calls), I started running into difficulties with how the SigningRequest and ResolvedSigningRequest objects were tied into the .transact call.

The scenario is this:

const session = new Session(/** args to create session */)

const transaction = {
    expiration: '2022-12-07T22:39:44',
    ref_block_num: 2035,
    ref_block_prefix: 2373626664,
    max_net_usage_words: 0,
    max_cpu_usage_ms: 0,
    delay_sec: 0,
    context_free_actions: [],
    actions: [
        {
            account: 'foo',
            name: 'bar',
            authorization: [
                {
                    actor: 'wharfkit1111',
                    permission: 'test',
                },
            ],
            data: '104208d9c1754de380b1915e5d268dca390500000000000004454f53000000001777686172666b6974206973207468652062657374203c33',
        },
    ],
    transaction_extensions: [],
}

const { signatures } = await session.transact(transaction, { broadcast: false })

3 things are being done here:

1. Manually create a session.
2. Have an already existing transaction you just want to sign.
3. Get the Session to sign without broadcasting and return the signature(s).

The problem I ran into is that despite a complete transaction being passed in, which is ready to be signed, this transaction being converted into an EOSIO Signing Request, the transaction being resolved, and then passed to the wallet plugin. The step where it resolves the transaction requires that the ABIs be loaded, to ensure any placeholder data can be detected and updated.

A detailed breakdown of this process is as follows:

1. The .transact method is taking in any number of transaction-like things (action, action array, transaction, esr string, etc)
2. The first thing that happens is whatever was passed in is converted into a SigningRequest.
3. The beforeSign hooks from the loaded TransactPlugin are run. They are run sequentially and take in a clone of the signing request. Each run potentially returns a new/modified SigningRequest - which is passed subsequent hook runs (allowing the chaining of hooks and plugins).
4. After any beforeSign complete, the SigningRequest is resolved and a ResolvedSigningRequest is created.
5. The ResolvedSigningRequest is now passed into the Session's WalletPlugin to sign. The SigningRequest isn’t used here because none of the placeholder values are resolved on it. If the resolve step wasn't done in the transact method, every WalletPlugin would be responsible for that operation themselves.
6. The WalletPlugin will return an array of signatures, and optionally, a ResolvedSigningRequest which was modified by the wallet. The WalletPlugin calling .sign() is the last time the transaction can be modified.
7. More plugins trigger, broadcast logic is applied, etc - none of which is relevant to this discussion.

Resolving the transaction doesn't seem like it could be removed from the transact call, but it's what ends up making the API calls.

A few ideas I've had:

• Adding a new method to the Session, like session.sign(transaction), which only accepts a TransactionType parameter. This is a separate path aside from .transact(), won't accept placeholder values, won't run plugins, and won't broadcast. It will set the allowModify flag to false as it asks the WalletPlugin to sign (so the plugin knows not to modify it). This new method will then just return an array of signatures.
• In the session.transact() calls where a full transaction (+tapos) is passed in, it'll be passed through a specific path which prevents any modifications (no placeholders). From the beginning of the call it will be represented as a ResolvedSigningRequest. Basically if an app calls transact with a completed transaction, we treat it as completed, instead of like normal in Step 6 above.
• Remove EOSIO Signing Request completely from the .transact() flow and use native Transaction, SignedTransaction, and PackedTransaction types throughout the entire code path. An ESR payload could still be passed into transact as a parameter (and immediately be resolved to a Transaction), the SigningRequest objects just wouldn't be used throughout the internals. We will lose the ability in plugins to attach metadata and probably a few other helpful features EOSIO Signing Request provides. We would however gain a much more simple data format for use in both WalletPlugin and TransactPlugin instances.

The first option feels like a cheap way out, just sidetracking the problem by adding a new method. The second option feels rigid and might cause some confusion. The third option seems like the most simple approach but goes against one of our design goals of wanting ESR to represent the internal payload structure throughout Wharf.

I'm not sure the best approach here, so I figured I'd jot down my thoughts here and spark up a discussion.

[aaroncox]

I'm going to go the "cheap way out" for the time being - though we may want to change this approach in the future. There's a new signTransaction method on each session, which accepts only full transactions and will request a signature from the given WalletPlugin. This method can be found here (and is pretty hacky):

https://github.com/wharfkit/session/blob/6f728877618968cfff56a97e2fa853d9fa5598f7/src/session.ts#L520

And an example of how its being used can be found here:

https://github.com/wharfkit/session/blob/6f728877618968cfff56a97e2fa853d9fa5598f7/test/tests/session.ts#L363-L407

const testSession = new Session(
    {
        chain: {
            id: '73e4385a2708e6d7048834fbc1079f2fabb17b3c125b146af438971e90716c4d',
            url: 'https://jungle4.greymass.com',
        },
        permissionLevel: 'account@permission',
        walletPlugin: new WalletPluginPrivateKey(
            '5Jtoxgny5tT7NiNFp1MLogviuPJ9NniWjnU4wKzaX4t7pL4kJ8s'
        ),
    },
    {
        fetch: mockFetch,
    }
)

// This is actually an eosio.token:transfer, with a renamed contract/action to break unittest caching
const transaction = {
    expiration: '2022-12-07T22:39:44',
    ref_block_num: 2035,
    ref_block_prefix: 2373626664,
    max_net_usage_words: 0,
    max_cpu_usage_ms: 0,
    delay_sec: 0,
    context_free_actions: [],
    actions: [
        {
            account: 'foo',
            name: 'bar',
            authorization: [
                {
                    actor: 'wharfkit1111',
                    permission: 'test',
                },
            ],
            data: '104208d9c1754de380b1915e5d268dca390500000000000004454f53000000001777686172666b6974206973207468652062657374203c33',
        },
    ],
    transaction_extensions: [],
}

// Retrieve the signature(s), do not use the transact method path.
const signatures = await testSession.signTransaction(transaction)