Open geoknee opened 1 year ago
From standup today:
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 abigen
so we can get under the hood and make it do what we want. See this https://github.com/ethereum/go-ethereum/pull/26782 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).
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.
Context
Currently,
go-nitro
accepts achainPk
(chain private key) through its CLI entrypoint. This is used to construct anEthChainService
which is (in turn) injected into thenode
constructor.. TheEthChainService
will bind an internaltxSigner
variable to thatchainPk
, 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:https://github.com/statechannels/go-nitro/blob/2b0bc7c5998abe021970704f448d030844a05928/node/engine/chainservice/chainservice.go#L85-L98
I propose that will split up the interface to have prepare transaction and send transaction:
Happily, we already have some abstract transaction types which are "without signatures": https://github.com/statechannels/go-nitro/blob/2b0bc7c5998abe021970704f448d030844a05928/protocols/interfaces.go#L29-L32
rpc
,node
andethchainservice
). 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.txSigner
with atxCaller
where appropriate inside the eth chain service. The only place where tx are signed will be within theSendTransaction
method.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.engine.executeSideEffects
function will always callPrepareTransaction
, but depending on the config (discussed above), will immediately callSendTransaction
(existing behaviour) or route the unsigned transaction back to the user.TransactionsToSign
field to theEngineEvent
struct: https://github.com/statechannels/go-nitro/blob/2b0bc7c5998abe021970704f448d030844a05928/node/engine/engine.go#L93-L105executeSideEffects
can return anEngineEvent
which should be merged with other events generated inside the engine handler functionsnode.transactionsToSign chan types.Transaction
( a newchan
which needs to be added).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.