[gateway api] Replace TransactionSigner with two steps call

[patrickkuo]

We are replacing TransactionSigner with two steps call.

The reason of this change is to simply the mechanism of acquiring signatures, TransactionSigner's signature callback is hard to implement for the rest server without using rest server callback. The new 2 steps structure will be much easier to implement across the rest boundaries.

Changes to the Gateway API

• Instead of passing in a transaction signer, we have now split the transaction process into 2 parts.
• All of the function in GatewayAPI which require signature will return TransactionSignatureRequest instead of executing the transaction, the function call will create the TransactionData, register it in the GatewayState and return a TransactionSignatureRequest object to the client.
• The client is expected to create a signature using their key, with the TransactionData or TransactionDigest included in the TransactionSignatureRequest and return it to the GatewayAPI using the execute_transaction function
• A new API method execute_transaction is added to execute the registered transaction.
• Added enum TransactionResponse, for different response type for different transaction kind.
• Added SplitCoin and MergeCoin to TransactionKind, so they can have their own response type.

[arun-koshy]

We will be passing this from the wallet application to the rest server. So just want to confirm, are both the signature and the sig_req.digest safe to transfer over the wire?

[patrickkuo]

yes it's cryptographically safe

[lxfind]

Sorry for the late review! Thanks for looking into simplifying the signature process.
I have two high-level questions about this change:

1. Can we live without adding two new variants in TransactionKind that's essentially special cases of MoveCall? It seems like you want to distinguish among the different request intent called on the GatewayAPI. For that, I believe the proper approach would be to introduce enums just for the various request types, which wraps the transaction data in it, so that you can keep the request kinds and transaction kinds separate (they can map to each other in arbitrary ways).
2. Given the direction we are going, it seems less and less meaningful for the GatewayAPI to actually receive requests on each individual kinds (move_call, transfer, publish and etc.) just to package them up and return the transaction for the caller to sign. Should the caller just package a transaction themselves, sign them and send to GatewayAPI? We could add a library for applications to use to package transactions through a list of standard APIs, so they could package the transaction locally. This is just a thought, may be something worth asking/flagging in the slack channel to get more eyes on this (and if we go this route, we don't even need to discuss (1)).

[patrickkuo]

Sorry for the late review! Thanks for looking into simplifying the signature process. I have two high-level questions about this change:

1. Can we live without adding two new variants in TransactionKind that's essentially special cases of MoveCall? It seems like you want to distinguish among the different request intent called on the GatewayAPI. For that, I believe the proper approach would be to introduce enums just for the various request types, which wraps the transaction data in it, so that you can keep the request kinds and transaction kinds separate (they can map to each other in arbitrary ways).

I try not to introduce yet another enum and layer, hence the changes in TransactionKind; If this is not desirable, I can add a new GatewayRequestType enum for this.

2. Given the direction we are going, it seems less and less meaningful for the GatewayAPI to actually receive requests on each individual kinds (move_call, transfer, publish and etc.) just to package them up and return the transaction for the caller to sign. Should the caller just package a transaction themselves, sign them and send to GatewayAPI? We could add a library for applications to use to package transactions through a list of standard APIs, so they could package the transaction locally. This is just a thought, may be something worth asking/flagging in the slack channel to get more eyes on this (and if we go this route, we don't even need to discuss (1)).

I have suggested this approach last time we discuss Gateway architecture, the conclusions I had after some thoughts are:

1, All of the transaction require ObjectRef as input, if we build the transaction on the application side, the application will need to do multiple calls to fetch the latest ObjectRef to build the transaction.

2, We will need to provide and maintain libs, potentially for many different languages to creating the transaction locally to ensure the serialisation will create the same bytes (we use BCS), otherwise it will result in different hash and invalid signature. In this PR I have included TransactionDigest in TransactionSignatureRequest, the application can use the digest bytes to create signature without BCS or Sui lib via rest.

[lxfind]

This worries me. It creates a way for people to DDos our storage: just keep sending tx without signing it, this table will continue to grow.
I assume this is added as an optimization to reduce the size of the second request? Although we will need to optimize it in some way, it may be better to leave that out for simplicity at the moment.

[patrickkuo]

This is not just an optimisation to reduce size, it also make the rest request a lot simpler, malicious actor will need to try different combination of objects to create unique transactions with different digest, so the grow of this map should be limited, having said that, I am happy to remove it for now to make it safer.

Thinking more about DDoS... current implementation of gateway will allow read access without proof of key ownership, which will allow DDoS and address_states can grow infinitely, we need to think about authentication in the gateway service.

[lxfind]

A user can submit a Move Call request, and simply mutating one pure argument from 0,1,... and can easily create infinite number of unique transactions?

[patrickkuo]

Oh of cause, I was only thinking about transfers 🤦🏻‍♂️, thanks!

[lxfind]

Also, you may consider moving the to_response functions to the gateway_responses.rs

[patrickkuo]

I have further simplified the changes!

• execute_transaction now takes Transaction object as input, which can be constructed on the client side.

• removed unsigned_tx from GatewayState as it is unsafe

• transfer_coin, publish ... methods in GatewayAPI now returns TransactionData
  (we don't really need these apis anymore as suggested by @lxfind , application can use TransactionData::new_transfer etc to create transaction data, will leave the refactoring to next PR as there are a lot of effort refactoring all the tests)

• Removed GatewayRequestType enum, request types can be determined by examining the TransactionData

• removed TransactionSigner

[lxfind]

Looking good. Thanks!