stampchain-io / btc_stamps

Bitcoin Stamps Indexer
https://stampchain.io
GNU Affero General Public License v3.0
9 stars 2 forks source link

Incorrect txlist_hash consensus hash #289

Closed blocknodehub closed 1 week ago

blocknodehub commented 1 month ago

Describe the bug Incorrect txlist_hash consensus hash for block 795000. Calculated afc5aa18eaeaaa598c1706e7fe6fa072a483797090f1c05274236f4171073b9b but expected e6b33fa56c627ebb96b4207113e70fb449ed86e05c77b43f2b1df3be7edf225b

To Reproduce This error is reported when fetching data from my own BTC node and then syncing to height 795000

Expected behavior I expect normal synchronisation.

Screenshots image

reinamora137 commented 1 month ago

what branch was this on? [edit] I do see in the screenshot that is on v 1.9.0 which is not a release?

i did see a mismatch recently as well, but on a much later block.

reinamora137 commented 1 month ago

for posterity. txlist hash mismatches were expected on releases prior to 1.8.2 because of how the tx_index value was computed coming through the multithreading. this should have been resolved, but will be investigating.

txlist_hash is the validation of txlist_content = str(valid_stamps_in_block)

this is also related to pending CI workflow to validate hashes in github actions : https://github.com/stampchain-io/btc_stamps/issues/new

reinamora137 commented 1 month ago

valid_stamps_in_block: List[ValidStamp] = []

class ValidStamp(TypedDict):
    stamp_number: int
    tx_hash: str
    cpid: str
    is_btc_stamp: bool
    is_valid_base64: bool
    stamp_base64: str
    is_cursed: bool
    src_data: str

I believe in the latest dev builds is_cursed and possibly is_valid_base64 (have not validated which ones) were forced to be a bool value to exclude prior null values which would change the hashes when comparing with the 1.8.2 production release (and the associated values in the check.py which are for 1.8.2). This change was to adjust for the schema in the API not expecting null for those values for cleanliness (however null values are currently ok so this isn't a required change - TBD). Will need to run some revalidations on this change for the next release and uddate the expected hashes in check.py and update the production db with these changes. the most critical comparison is that the stamp numbers match so perhaps we adjust the fields in ValidStamp to only include the stamp_number and tx_hash and likely add the tx_index just in case there are future changes that would impact the structure of the other fields in that dict.

For dev testing the values in check.py can be removed to complete indexing and other tests until this is cleaned up and pushed into the next production release.

Running a full re-index on 1.8.2 to validate there aren't any problems there when compared to pruduction before we proceed.

reinamora137 commented 1 month ago

i'm thinking this reversion on the dict structure should fix the issue and keep linting in line. to be tested.

https://github.com/stampchain-io/btc_stamps/commit/43d6cd11e6943304d8365305079270f0f48dd559

reinamora137 commented 1 month ago

openstamp is also experiencing issues with block 835000 and the txlist hash on 1.8.2 tag. will need to adjust for 1.8.3 release pending. a good time to adjust ValidStamp

reinamora137 commented 1 month ago

the consensus hash issue was not resurfaced on 2 additional indexes of main 1.8.2 on this end. this will be resolved in an upcoming canary release if hashes need to be modified.

blocknodehub commented 1 month ago

I have a new problem: Error in minting operations: 'pepe' image

reinamora137 commented 1 week ago

the block hashes have been revised in the most current dev branch and will the the hashes for the next release.

reinamora137 commented 1 week ago

I have a new problem: Error in minting operations: 'pepe' image

i have not been able to replicate this error. if it still persists please open a new issue