Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for the consumer to sign their own chain transactions #1416

Open
geoknee opened this issue Jul 10, 2023 · 3 comments
Open

Allow for the consumer to sign their own chain transactions #1416

geoknee opened this issue Jul 10, 2023 · 3 comments
Labels

Comments

@geoknee
Copy link
Contributor

geoknee commented Jul 10, 2023

Context

Currently, go-nitro accepts a chainPk (chain private key) through its CLI entrypoint. This is used to construct an EthChainService which is (in turn) injected into the node constructor.. The EthChainService will bind an internal txSigner variable to that chainPk, so has the ability to sign transactions with that key.

This way, when go-nitro decides it is safe to launch an on-chain transaction, a side effect will be generated by a protocol / objective "crank", routed to the chain service, signed and launched to the blockchain by the chain service.

Problem

Users may not want to trust go-nitro to sign and send transactions with their layer 1 blockchain private key.

Solution

An alternative pattern we could support is having go-nitro prepare a transaction and send on or return to the user to sign and submit.

We would like to be able to preserve the current behaviour, and switch depending on a command line parameter / toml config.

Detail

The existing ChainService interface implies a blend of:

  • "free" contract calls (no signing required)
  • transaction preparation / knowledge of smart contract API
  • transaction signing

type ChainService interface {
// EventFeed returns a chan for receiving events from the chain service.
EventFeed() <-chan Event
// SendTransaction is for sending transactions with the chain service
SendTransaction(protocols.ChainTransaction) error
// GetConsensusAppAddress returns the address of a deployed ConsensusApp (for ledger channels)
GetConsensusAppAddress() types.Address
// GetVirtualPaymentAppAddress returns the address of a deployed VirtualPaymentApp
GetVirtualPaymentAppAddress() types.Address
// GetChainId returns the id of the chain the service is connected to
GetChainId() (*big.Int, error)
// Close closes the ChainService
Close() error
}

I propose that will split up the interface to have prepare transaction and send transaction:

PrepareTransaction(protocols.ChainTransaction) (types.Transaction, error) // has nil signature
SendTransaction(types.Transaction) error // requires non-nil signature, returns an error otherwise

Happily, we already have some abstract transaction types which are "without signatures":

type DepositTransaction struct {
ChainTransaction
Deposit types.Funds
}

  • It should be optional to set a nonzero chain pk throughout the stack (from rpc, node and ethchainservice). I suggest a nil key should imply "do not send transactions, but return them over the API". Whether we do it that way or with more explicit options / flags, it must be clearly documented.
  • We should replace the txSigner with a txCaller where appropriate inside the eth chain service. The only place where tx are signed will be within the SendTransaction method.
  • We use automatically generated Go bindings (using abigen to send transactions). This makes it difficult to split out the transaction preparation from the signing and sending. So we are probably forced to craft the "raw transactions" ourselves and to not lean on the go bindings at all 😬 . Hopefully not too difficult because we can copypasta code from the existing go bindings.
  • The engine.executeSideEffects function will always call PrepareTransaction, but depending on the config (discussed above), will immediately call SendTransaction (existing behaviour) or route the unsigned transaction back to the user.
  • "routing the unsigned transaction back to the user" can be achieved as follows:
    • add a TransactionsToSign field to the EngineEvent struct:
      type EngineEvent struct {
      // These are objectives that are now completed
      CompletedObjectives []protocols.Objective
      // These are objectives that have failed
      FailedObjectives []protocols.ObjectiveId
      // ReceivedVouchers are vouchers we've received from other participants
      ReceivedVouchers []payments.Voucher
      // LedgerChannelUpdates contains channel info for ledger channels that have been updated
      LedgerChannelUpdates []query.LedgerChannelInfo
      // PaymentChannelUpdates contains channel info for payment channels that have been updated
      PaymentChannelUpdates []query.PaymentChannelInfo
      }
    • executeSideEffects can return an EngineEvent which should be merged with other events generated inside the engine handler functions
    • the new field should be handled inside the node:
      func (n *Node) handleEngineEvents(ctx context.Context) {
      and sent on node.transactionsToSign chan types.Transaction( a new chan which needs to be added).
    • This chan needs to be threaded over the RPC layer in the same way as the other event streams. See
      func (rc *RpcClient) LedgerChannelUpdatesChan(ledgerChannelId types.Destination) <-chan query.LedgerChannelInfo {

Unknowns

Should go-nitro accept signed transactions so it can send them? Or do we assume the caller can send them (e.g. through metamask).

Proof

We should do a manual integration test between two go-nitro nodes. One can run headless and manage its own chainPk. The other is run headful using the go-nitro GUI, which should be wired up to route transactions to metamask through the browser (like so). If we can open and fund a ledger channel between those two nodes, we will have achieved our aim.

@geoknee geoknee added the yellow label Jul 10, 2023
@geoknee geoknee changed the title Spike: investigate removing chainPk from go-nitro Allow for the consumer to sign their own chain transactions Jul 18, 2023
@geoknee
Copy link
Contributor Author

geoknee commented Jul 18, 2023

From standup today:

  1. Instead of rewriting the code which is currently autogenerated by abigen, we could instead pass a different tx.Opts.Signer function. This would encapsulate sending the raw transaction back to the user and having the user return a signed transaction. This would mean go-nitro is still responsible for sending the transaction. This might be a bit messy when it comes to managing concurrency / not blocking
  2. We could fork abigen so we can get under the hood and make it do what we want. See this ABIGEN v2 ethereum/go-ethereum#26782

@geoknee
Copy link
Contributor Author

geoknee commented Jul 19, 2023

For the abigen v2 route see the sketch code here: https://github.com/statechannels/go-nitro/compare/abigenv2?expand=1#diff-108f221519228166f68fe014cb4428e8cb8cb434a564a008109ae9c611d60a2cR165-R177

The real advantage is that breaking changes to our solidity code API will flag immediately in the go-nitro codebase as a compiler error.

The alternative is to hand-roll transaction preparation. It's not difficult, and in fact we are already doing something like this when parsing events (of course it has the same downsides #1417).

@geoknee
Copy link
Contributor Author

geoknee commented Jul 19, 2023

If the transaction is to be prepared by go-nitro and then signed by the consumer, it is not obvious which party is responsible for setting gas costs and account nonces. If the consumer ultimately is using some wallet like metamask, then metamask will overwrite these values anyway. I think probably go-nitro should leave these fields null / blank for the consumer to fill in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant