go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

Make `transaction.setDecoded` public

Open mempirate opened this issue 3 years ago • 1 comments

Rationale

Context: using go-ethereum as a library to work with ethereum objects.

Currently, when constructing a new Transaction object, the only way to do that using go-ethereum as a library is using types.NewTx(inner TxData). This calls inner.copy(), which (as I understand it) is mainly used to make sure all the fields are initialized. The problem is that this accounts for quite a few allocations, which are easily avoidable if the caller knows that all fields will be initialized anyway.

Implementation

The solution would be to directly allow callers to use setDecoded, so making it a public method. I can open a PR for making it public unless I'm not understanding something correctly. In that case please let me know.

mempirate avatar Sep 20 '22 08:09 mempirate

Another method that could be introduced and might be clearer in the API is UnsafeNewTx(inner TxData), which does the same as NewTx(inner TxData) but without deep copying the TxData.

mempirate avatar Sep 21 '22 08:09 mempirate

Hi. I understand that you are looking at this API from an efficiency perspective. We are more focused on correctness with this API. types.Transaction and types.Block are meant to be 'immutable' and will never share memory with the objects passed to the constructor. The initialization of missing fields is more like a side-benefit.

Other mechanisms such as the inline tx hash, tx sender and size cache are built on this immutability guarantee, and their semantics could be broken in case of an unexpected modification. For this reason, we will not accept an 'unsafe' constructor in package types.

fjl avatar Sep 22 '22 13:09 fjl

Okay, that makes perfect sense, thank you for clarifying. I'll close this issue!

mempirate avatar Sep 22 '22 20:09 mempirate