Skip to content

Conversation

@nasdf
Copy link
Contributor

@nasdf nasdf commented Jul 31, 2021

Hello,

This PR should make it easier to sign EIP-712 typed data from the accounts.Wallet API.

var wallet accounts.Wallet
var account accounts.Account
var typedData signer.TypedData

data, _ := json.Marshal(typedData)

wallet.SignData(account, accounts.MimetypeTypedData, data)

@nasdf nasdf requested a review from holiman as a code owner July 31, 2021 18:38
@holiman
Copy link
Contributor

holiman commented Aug 17, 2021

Not sure I follow. Why can't you use this existing method?

func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, error) {
	signature, _, err := api.signTypedData(ctx, addr, typedData, nil)
	return signature, err
}

@holiman
Copy link
Contributor

holiman commented Aug 17, 2021

Ah, ok, it's not available on the interface. So then I think you could still avoid all the copy-paste code, and just call that method from inside the switch... ?

@nasdf
Copy link
Contributor Author

nasdf commented Aug 17, 2021

Ah, ok, it's not available on the interface. So then I think you could still avoid all the copy-paste code, and just call that method from inside the switch... ?

That would be a much better approach but determineSignatureFormat is expecting a SignDataRequest return value.
If you think a better approach is to add it to the wallet interface I'd be happy to try that route instead.

@holiman
Copy link
Contributor

holiman commented Mar 22, 2022

We already have SignGnosisSafeTx, isn't that basically the same thing?

@nasdf
Copy link
Contributor Author

nasdf commented Mar 22, 2022

We already have SignGnosisSafeTx, isn't that basically the same thing?

It does do something similar, but the goal of this PR is to make meta transactions possible from the accounts.Wallet interface.

As a user of go-ethereum as a library there’s currently no straightforward way to sign meta transactions in a generic way that would work with key stores, ledger, etc.

@holiman
Copy link
Contributor

holiman commented Nov 23, 2022

Superseded by #26241

@holiman holiman closed this Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants