Fallback to getblock when getrawtransaction fails on Genesis TX #119
Conversation
|
Fixes the issue described, but I will let other electrs champions actually review the PR. electrsmempool backend |
|
How about when using dark mode |
get out |
|
Approach Nack, (Too magical) I am not in favor of this solution because at the end of the day this only solves for a single network Mainnet. For an application that supports various networks, Signet, Testnet, etc ... This would lead to inconsistent behavior across use cases. |
|
why don't we fetch the whole genesis block, extract the coinbase transaction, and then cache it somewhere? |
|
|
Just don't use light mode and this isn't an issue |
junderw
force-pushed
the
junderw/hardcode-genesis-tx
branch
from
d849d02 to
523268e
Compare
junderw
changed the title
portlandhodl
left a comment
portlandhodl
left a comment
There was a problem hiding this comment.
I am not in a position to approve or reject anything with that said here are my thoughts.
On the level of the bigger picture. My preference is not to overload the underlying forwarding calls to Bitcoin Core instead wrapping or defining a new fn()/trait with the overloaded functionality. I don't maintain anything here so it's just my opinion.
| #[cfg(not(feature = "liquid"))] | ||
| Err(e) | ||
| if e.to_string().contains( | ||
| "genesis block coinbase is not considered an ordinary transaction", |
There was a problem hiding this comment.
Common issue with Bitcoin Core is that errors are not enumerated, the error number value is considered more stable.
|
|
||
| // Assert that there's only 1 tx, the previous block hash is all zeroes, and not using verbose | ||
| if verbose | ||
| || block.txdata.len() != 1 |
There was a problem hiding this comment.
Nit: Genesis blocks currently always have 1 TXN, but might not in the future + peoples own Regtest instances. A greater than 0 check would be better and still gives you safe access to [] accessor.
Edit: Because of the way the merkleroot in the header works, There isn't a need to put gaurdrails around accessing element 0 because at minimum a block will always have a single transaction in your array.
| // Assert that there's only 1 tx, the previous block hash is all zeroes, and not using verbose | ||
| if verbose | ||
| || block.txdata.len() != 1 | ||
| || block.header.prev_blockhash != BlockHash::default() |
There was a problem hiding this comment.
Nit: Not needed if you already have genesis TXN which is always a block at height 0, so previous hash is always 0...0
In light mode, the block summary /txs for genesis block is borked thanks to RPC error.