-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Confirmations undefined on response from JsonRpcProvider.sendTransaction #1706
Comments
Yes the @ricmoo Either we can make confirmations optional or better it can be initialized to 0 if tx is pending and 1 if just confirmed (ganache/hardhat). What do you think? |
Yes, I should be initializing it to 0. I will still let that be the result even on Ganache since the updating poll will fill it in. I don't like making special exceptions, especially for test environments, but will certainly get things initialized properly in the next release. :) |
Actually, I can't find any paths which result in |
When we do (It appears from source code that it should work, but not sure why it is not) Can you try to reproduce the following > await w.sendTransaction({})
Object
chainId: 42
data: "0x"
from: "0xC8e1F3B9a0CdFceF9fFd2343B943989A22517b26"
gasLimit: e {_hex: "0xcf08", _isBigNumber: true}
gasPrice: e {_hex: "0x0147d35700", _isBigNumber: true}
hash: "0x58cb60724ffd8b00b7991df86ea913848a8fce4434bd9c9d201495ac73738acf"
nonce: 1885
r: "0x503ab8764461fc8fcfc5a8b1ba51b5adc7c90674d9039a79695999d0271c8816"
s: "0x39b322b482f42c91439a45a45113a64996661463f5414fcf2a85c6b601b3fa24"
to: null
type: null
v: 119
value: e {_hex: "0x00", _isBigNumber: true}
wait: function(t,o)
Object Prototype It really doesnt contain confirmations for some reason. While when I manually tried getTransaction -> _wrapTransaction, it works. |
Weird… Thanks! I’ll use that to reproduce it. :) |
I was able to reproduce it. Working on it now. :) |
I’ve fixed it locally, but have a few other changes I am working on. It will go out with the next release. :) |
This is fixed in 5.4.2. Let me know if you still have any issues. :) |
Closing this now, as it should be resolved. If not though, please re-open. Thanks! :) |
@ricmoo I'm not sure I follow this "fix". I see that the value now gets initialized to zero, but it never changes. What's the reason it doesn't get populated with the actual number of block confirmations? (Edited to remove inaccurate comment about optional type declaration) |
@fubar A confirmations of 0 indicates the block is not yet mined (once it is in a block, it has 1 confirmation). A block that has not been mined has no timestamp, because that is the time at which it was mined (according to the block header of its block). Nearly every object in ethers is meant to be immutable. It doesn’t update the object, as the object represents a snapshot of the data at the time it was collected. You need to call the method again for a newer snapshot. Or use the Make sense? |
@ricmoo got it, thanks for the explanations. What's the reason |
@fubar It isn’t part of the official response API of the JSON-RPC, which a lot of the ethers porcelain API was originally modelled after. Deviating would incur additional costs, since I would have to call Why isn’t it included there? My guess is that in Geth it would have meant another leveldb lookup against the disk, so they decided. It to, but I don’t know for certain. Just a guess. :) |
@ricmoo I see, makes sense. Cheers! |
Describe the bug
TransactionResponse
declaredconfirmations
as required when it appears to be optional. I am hitting some public MATIC mainnet nodes, so not 100% sure if this is a typing issue brought about by the differences in JsonRPC there. I am hittingethers.providers.JsonRpcProvider.sendTransaction
Reproduction steps
Console output:
Environment:
Node:
v16.2
Ethers:
v5.3.1
Search Terms
Often similar issues have come up before. Include any search terms you have tried in this repository's Issues (including closed issues) and "Discussions", so if there are matching issues, we can be sure to add those keywords and link this issue to it, making it easier for people to find in the future.
The text was updated successfully, but these errors were encountered: