Boilertalk / Web3.swift

A pure swift Ethereum Web3 library
MIT License
643 stars 189 forks source link

EthereumTransaction optional values #22

Closed pixelmatrix closed 6 years ago

pixelmatrix commented 6 years ago

What do you think about making some of EthereumTransaction's values optional? Our SDK signs remotely so we'd like to be able to create an EthereumTransaction object that only includes the necessary keys for the standard eth_sendTransaction call.

Our ideal implementation would have the following as optional:

One way to keep strict requirements here could be splitting it into EthereumTransaction and EthereumSignedTransaction with some base protocol.

koraykoska commented 6 years ago

The EthereumTransaction as of now is specifically made for signing. I don't think we can change it to fit both the signable and not signable transactions so splitting them into two different unrelated structs seems like the way to go.
A few things to note:

pixelmatrix commented 6 years ago

Okay that sounds good to me. I’ll work on a proposal today.

On May 15, 2018, at 6:09 PM, Koray Koska notifications@github.com wrote:

The EthereumTransaction as of now is specifically made for signing. I don't think we can change it to fit both the signable and not signable transactions so splitting them into two different unrelated structs seems like the way to go. A few things to note:

to should actually be Optional in the signable transaction as, like you said, creating a contract requires you to have no receiver. That's something I missed there. The non-signable transaction shouldn't be RLPItemConvertible as it can't be rlp encoded. It should also not have a sign function. v, r, s and chainId are not relevant for the non-signable transaction and can be omitted As stated in the json rpc documentation for eth_sendTransaction, the only two fields which must be set are from and data (data can have an empty array as a default value though). All other fields can be optional. I am not sure about the optional fields of the signable transaction. Do you know what fields have to be set in the final signed transaction? I haven't tried omitting some of them yet. to still have to be optional though. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

pixelmatrix commented 6 years ago

This was fixed with #24