Minimize Signer interface and document Sign (#2740)

wallet/chain/c/signer.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 
 	"github.com/ava-labs/coreth/plugin/evm"
+	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/ids"
@@ -20,7 +21,6 @@ import (
 	"github.com/ava-labs/avalanchego/vms/secp256k1fx"
 
 	stdcontext "context"
-	ethcommon "github.com/ethereum/go-ethereum/common"
 )
 
 const version = 0
@@ -37,16 +37,23 @@ var (
 )
 
 type Signer interface {
-	SignUnsignedAtomic(ctx stdcontext.Context, tx evm.UnsignedAtomicTx) (*evm.Tx, error)
+	// SignAtomic adds as many missing signatures as possible to the provided
+	// transaction.
+	//
+	// If there are already some signatures on the transaction, those signatures
+	// will not be removed.
+	//
+	// If the signer doesn't have the ability to provide a required signature,
+	// the signature slot will be skipped without reporting an error.
 	SignAtomic(ctx stdcontext.Context, tx *evm.Tx) error
 }
 
 type EthKeychain interface {
 	// The returned Signer can provide a signature for [addr]
-	GetEth(addr ethcommon.Address) (keychain.Signer, bool)
+	GetEth(addr common.Address) (keychain.Signer, bool)
 	// Returns the set of addresses for which the accessor keeps an associated
 	// signer
-	EthAddresses() set.Set[ethcommon.Address]
+	EthAddresses() set.Set[common.Address]
 }
 
 type SignerBackend interface {
@@ -67,11 +74,6 @@ func NewSigner(avaxKC keychain.Keychain, ethKC EthKeychain, backend SignerBacken
 	}
 }
 
-func (s *txSigner) SignUnsignedAtomic(ctx stdcontext.Context, utx evm.UnsignedAtomicTx) (*evm.Tx, error) {
-	tx := &evm.Tx{UnsignedAtomicTx: utx}
-	return tx, s.SignAtomic(ctx, tx)
-}
-
 func (s *txSigner) SignAtomic(ctx stdcontext.Context, tx *evm.Tx) error {
 	switch utx := tx.UnsignedAtomicTx.(type) {
 	case *evm.UnsignedImportTx:
@@ -150,6 +152,11 @@ func (s *txSigner) getExportSigners(ins []evm.EVMInput) [][]keychain.Signer {
 	return txSigners
 }
 
+func SignUnsignedAtomic(ctx stdcontext.Context, signer Signer, utx evm.UnsignedAtomicTx) (*evm.Tx, error) {
+	tx := &evm.Tx{UnsignedAtomicTx: utx}
+	return tx, signer.SignAtomic(ctx, tx)
+}
+
 // TODO: remove [signHash] after the ledger supports signing all transactions.
 func sign(tx *evm.Tx, signHash bool, txSigners [][]keychain.Signer) error {
 	unsignedBytes, err := evm.Codec.Marshal(version, &tx.UnsignedAtomicTx)

wallet/chain/c/wallet.go

@@ -140,7 +140,7 @@ func (w *wallet) IssueUnsignedAtomicTx(
 ) (*evm.Tx, error) {
 	ops := common.NewOptions(options)
 	ctx := ops.Context()
-	tx, err := w.signer.SignUnsignedAtomic(ctx, utx)
+	tx, err := SignUnsignedAtomic(ctx, w.signer, utx)
 	if err != nil {
 		return nil, err
 	}

wallet/chain/p/signer.go

@@ -16,7 +16,14 @@ import (
 var _ Signer = (*txSigner)(nil)
 
 type Signer interface {
-	SignUnsigned(ctx stdcontext.Context, tx txs.UnsignedTx) (*txs.Tx, error)
+	// Sign adds as many missing signatures as possible to the provided
+	// transaction.
+	//
+	// If there are already some signatures on the transaction, those signatures
+	// will not be removed.
+	//
+	// If the signer doesn't have the ability to provide a required signature,
+	// the signature slot will be skipped without reporting an error.
 	Sign(ctx stdcontext.Context, tx *txs.Tx) error
 }
 
@@ -37,11 +44,6 @@ func NewSigner(kc keychain.Keychain, backend SignerBackend) Signer {
 	}
 }
 
-func (s *txSigner) SignUnsigned(ctx stdcontext.Context, utx txs.UnsignedTx) (*txs.Tx, error) {
-	tx := &txs.Tx{Unsigned: utx}
-	return tx, s.Sign(ctx, tx)
-}
-
 func (s *txSigner) Sign(ctx stdcontext.Context, tx *txs.Tx) error {
 	return tx.Unsigned.Visit(&signerVisitor{
 		kc:      s.kc,
@@ -50,3 +52,12 @@ func (s *txSigner) Sign(ctx stdcontext.Context, tx *txs.Tx) error {
 		tx:      tx,
 	})
 }
+
+func SignUnsigned(
+	ctx stdcontext.Context,
+	signer Signer,
+	utx txs.UnsignedTx,
+) (*txs.Tx, error) {
+	tx := &txs.Tx{Unsigned: utx}
+	return tx, signer.Sign(ctx, tx)
+}

wallet/chain/p/wallet.go

@@ -495,7 +495,7 @@ func (w *wallet) IssueUnsignedTx(
 ) (*txs.Tx, error) {
 	ops := common.NewOptions(options)
 	ctx := ops.Context()
-	tx, err := w.signer.SignUnsigned(ctx, utx)
+	tx, err := SignUnsigned(ctx, w.signer, utx)
 	if err != nil {
 		return nil, err
 	}

wallet/chain/x/signer.go

@@ -15,7 +15,14 @@ import (
 var _ Signer = (*signer)(nil)
 
 type Signer interface {
-	SignUnsigned(ctx stdcontext.Context, tx txs.UnsignedTx) (*txs.Tx, error)
+	// Sign adds as many missing signatures as possible to the provided
+	// transaction.
+	//
+	// If there are already some signatures on the transaction, those signatures
+	// will not be removed.
+	//
+	// If the signer doesn't have the ability to provide a required signature,
+	// the signature slot will be skipped without reporting an error.
 	Sign(ctx stdcontext.Context, tx *txs.Tx) error
 }
 
@@ -35,11 +42,6 @@ func NewSigner(kc keychain.Keychain, backend SignerBackend) Signer {
 	}
 }
 
-func (s *signer) SignUnsigned(ctx stdcontext.Context, utx txs.UnsignedTx) (*txs.Tx, error) {
-	tx := &txs.Tx{Unsigned: utx}
-	return tx, s.Sign(ctx, tx)
-}
-
 func (s *signer) Sign(ctx stdcontext.Context, tx *txs.Tx) error {
 	return tx.Unsigned.Visit(&signerVisitor{
 		kc:      s.kc,
@@ -48,3 +50,12 @@ func (s *signer) Sign(ctx stdcontext.Context, tx *txs.Tx) error {
 		tx:      tx,
 	})
 }
+
+func SignUnsigned(
+	ctx stdcontext.Context,
+	signer Signer,
+	utx txs.UnsignedTx,
+) (*txs.Tx, error) {
+	tx := &txs.Tx{Unsigned: utx}
+	return tx, signer.Sign(ctx, tx)
+}

wallet/chain/x/wallet.go

@@ -286,7 +286,7 @@ func (w *wallet) IssueUnsignedTx(
 ) (*txs.Tx, error) {
 	ops := common.NewOptions(options)
 	ctx := ops.Context()
-	tx, err := w.signer.SignUnsigned(ctx, utx)
+	tx, err := SignUnsigned(ctx, w.signer, utx)
 	if err != nil {
 		return nil, err
 	}