Closed antazoey closed 2 months ago
antazoey
commented
2 years ago Also note that I need to have this transaction encoded while it is unsigned because I am sending it off to my Ledger device to get signed and the device expects the transaction to be RLP encoded.
kclowes
commented
2 years ago Thanks for raising this. Allowing an unsigned DynamicFeeTransaction looks like it's a gap in our functionality. I'll leave this open to track.
The hash I am getting does not seem right at all
What value do you expect?
I did some testing, and found a couple of workarounds:
If you can do a non-dynamic fee transaction, there is an UnsignedTransaction class that you can use to hash a transaction dictionary. Note that you'd have to also remove the chainId. So something like:
txn = {'to': b'', 'value': 0, 'data': b'...', 'nonce': 0, 'gas': 537114, 'gasPrice': 21000}
UnsignedTransaction.from_dict(txn).hash() # returns HexBytes('0x565e97e6eda3505dd4535c89c2eff01b3a49fee8ff4f46b7035f2a61da1e3fc7')If you do want to keep the Dynamic fee bit, it looks like you may have already found serializable_unsigned_transaction_from_dict. Use at your own risk since it's in _utils, there are no guarantees on the API staying the same. That being said, I don't foresee changes in the near future. Something like:
from eth_account._utils.legacy_transactions import serializable_unsigned_transaction_from_dict
h = serializable_unsigned_transaction_from_dict(txn).hash() # returns b' U:\xe9\x00\xe3O\xfc\\>\x82\x92\x97\xaf9\x9d\x1a\xed\x0f)\xc6%\xbf\\\x08\xc4\x95\t\xce.\xcc7', same as yours above
HexBytes(h) # returns HexBytes('0x20553ae900e34ffc5c3e829297af399d1aed0f29c625bf5c08c49509ce2ecc37')Hello @kclowes ! Thank you for the response and for looking this so much :)
What value do you expect?
I am not 100% sure on the exact value, however it does not seem like enough bytes. All the examples I have seen were 98 characters long, and the one I am getting is 66 characters long, so that made me think that this is not the right hash I need.
Here is an example in the Ledger app-ethereum of an already-encoded EIP-1559 hash ready to be signed: https://github.com/LedgerHQ/app-ethereum/pull/273/files
Also, I am working with Mike Shultz a little and testing out his PR: https://github.com/LedgerHQ/app-ethereum/pull/273/files
Setting breakpoints when using that lib, I can see the hashes look like the example in appe-ethereum.
The hashes I get from eth-account here are not accepted by the Ledger device either.
And unsigned transactions are working great, thanks for the warning on the usage of the protected _utils namespace.
kclowes
commented
2 years ago Hi :wave: ! No problem! I'm a little confused. To clarify, the UnsignedTransaction is working correctly but needs legacy gas fields. But you think DynamicFeeTransaction.from_dict(txn).hash() returns the wrong value?
The string you linked to in that PR (https://github.com/LedgerHQ/app-ethereum/pull/273/files) looks to me like an rlp encoding, rather than a keccak hash. A keccak hash is what is returned from WhateverTransaction.from_dict(txn).hash(). Is the value you're handing to the Ledger an rlp encoded transaction or a keccak hash of the rlp encoded transaction? If it's the rlp encoded transaction, you may be able to use pyrlp directly. Docs to encode custom objects are here. Let me know if (what) I'm misunderstanding!
antazoey
commented
2 years ago Is the value you're handing to the Ledger an rlp encoded transaction or a keccak hash of the rlp encoded transaction?
Yes, I am looking for the RLP.
That explains my confusion!
I was definitely the one with the misunderstanding but now I know. I can try to use the pyrlp lib.
This ticket can probably be closed, unless anything can come out of this.
I do find serialized_unsigned_transaction_from_dict() to be a bit strange because the legacy txn is returns is RLP encode-able but the typed transaction it returns is not (no sedes).
kclowes
commented
2 years ago Glad you figured it out :boom:!
I'm going to leave this open for now because there are some inconsistencies I'd like to look into, like returning an RLP encodable TypedTransaction, returning hash as either HexBytes or a bytestring consistently.
antazoey
commented
2 years ago It was weird because these transaction classes look like RLP encode-able transactions, but yeah I think they just assume signed? I am not sure! But I can use the unsigned legacy transaction in rlp.encode() but I can't use the unsigned EIP-1559 transactions that way.
For now, I just made my own rlp objects, but they look a like the ones in this package, so I am not sure what exactly is going on. Is this just because of the v r s fields?
kclowes
commented
2 years ago Yeah, I think the TypedTransactions do assume they're signed. I don't see a way to work with unsigned TypedTransactions, but I also only have surface-level knowledge of this part of the library, so I could be wrong. We do some mucking around with the access list parameters (converting them from a dict to a list and vice versa) and so I wonder if the timing of that is the problem - maybe they're getting passed to rlp as a dict which doesn't have sedes?
fselmo
commented
2 years ago 💯 ... Thanks for the conversation around this.
I think there's a lot to be improved in this process for sure. Having recently dived into rlp encoding for typed transactions, it was very difficult to navigate as it currently stands. Might be nice to separate these different ideas into bite-sized issues to better track them. I can start doing that and touch base to see if I missed anything.
fselmo
commented
2 years ago One thing that could currently work, and I think is what you're looking for unless I'm totally mistaken, is... using your example above:
import rlp
from eth.vm.forks.london.transactions import LondonTransactionBuilder, DynamicFeeTransaction
# notice the snake case since it's a py-evm model :(
tx = {
'chain_id': 61,
'to': b'',
'value': 0,
'data': b'`\x80`@R4\x80\x15a\x00\x10W`\x00\x80\xfd[P3`\x00\x00\x00...,
'access_list': [],
'nonce': 0,
'gas': 537114,
'max_priority_fee_per_gas': 875000000,
'max_fee_per_gas': 0,
# notice no `type` here since that would raise:
# 'type': 2
}
unsigned_tx = LondonTransactionBuilder.new_unsigned_dynamic_fee_transaction(**tx)
just_the_rlp_part = rlp.encode(unsigned_tx) # rlp(tx) -- in bytes
type_with_rlp = unsigned_tx.get_message_for_signing() # 0x02 || rlp(tx) -- in bytes
pacrob
commented
2 months ago Closing as stale
What was wrong?
An unsigned transaction's hash is wrong and not signable
My transaction does not have V, R, or S (it is unsigned). It also does not have any access lists. The tests and docs are not helping either:
test_typed_transaction::test_hashuses already signed transactionsThe hash I am getting does not seem correct because of its length and because the Ledger dongle does not accept it
MORE
This is my transaction dict:
This is the return value from
hash():Environment