Delegate digest generation to the auth module.

[containerman17]

Current implementation: The transaction calls its Digest method, which returns []byte. Then, chain.AuthFactory.Sign(msg []byte) signs the resulting bytes of the digest.

Proposal: Change to chain.AuthFactory.Sign(tx *Transaction) where chain.AuthFactory calls the Digest method instead of the transaction.

Similar changes for chain.AuthBatchVerifier.Add and chain.Auth.Verify.

[aaronbuchwald]

I think this is better as is since it is more general. This would couple the auth package directly to the *chain.Transaction, which I think it's desirable to avoid.

If an auth package needs the transaction details, I'd suggest we use an interface like:

type Byter interface {
	Bytes() []byte
}

and handle it within the auth package for that special case.

[containerman17]

@aaronbuchwald Unfortunately, our EIP712 implementation requires almost all the data from the chain.Transaction struct:

• tx.Base, with its ChainID, MaxFee, and Timestamp fields
• tx.Actions, which is an array of chain.Action

Other authentication methods do need the Digest() method, though.

Regarding the coupling of auth structs to chain.Transaction, I don't see an issue. If you want to customize a chain.Transaction object, it should become just an interface (see below). Almost no code changes would be needed.

type Transaction interface {
    GetBase() chain.Base
    GetActions() []chain.Action
    Bytes() []byte
    Sign(...) *Transaction, error
    Verify(...) error
}

But yeah, I have second thoughts about it. My solution doesn't look as neat as I would like if we end up making chain.Transaction an interface.

By the way, auth is used only in transaction signing/verifying, nothing else.

[aaronbuchwald]

@aaronbuchwald Unfortunately, our EIP712 implementation requires almost all the data from the chain.Transaction struct:

• tx.Base, with its ChainID, MaxFee, and Timestamp fields
• tx.Actions, which is an array of chain.Action

Other authentication methods do need the Digest() method, though.

Regarding the coupling of auth structs to chain.Transaction, I don't see an issue. If you want to customize a chain.Transaction object, it should become just an interface (see below). Almost no code changes would be needed.

type Transaction interface {
    GetBase() chain.Base
    GetActions() []chain.Action
    Bytes() []byte
    Sign(...) *Transaction, error
    Verify(...) error
}

But yeah, I have second thoughts about it. My solution doesn't look as neat as I would like if we end up making chain.Transaction an interface.

By the way, auth is used only in transaction signing/verifying, nothing else.

Hmm, I think it's a good thing for the auth package to be decoupled from the transaction type. It's not a large change, but I'd prefer not to couple the auth package to transactions if it can be avoided since it makes it more versatile and generally seems like a reasonable line to draw.

If we passed an interface in, then the EIP-712 package could cast to the original transaction type and others could call the Bytes() function, but even that seems less than ideal.

I don't want to make too large of a change to fit EIP-712 here either. Let's discuss offline and then post the result here.

[containerman17]

I couldn't use the Bytes() method because it is already utilized for storing bytes of a signed transaction. Instead, I used the existing Digest() method behind an interface:

type DigestProvider interface {
    Digest() ([]byte, error)
}

I'll test this version with manual casting for EIP712 next.

[containerman17]

Not currently necessary. Closing.