Skip to content
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

feat: allow parsing Bitcoin deposit memo with inscription and add E2E test for parsing #2727

Closed
wants to merge 48 commits into from

Conversation

bitSmiley
Copy link
Contributor

@bitSmiley bitSmiley commented Aug 16, 2024

Description

Adding e2e test for detecting memo from inscription.

The current btcutil does not have enough tapscript support, in this case, a nodejs sidecar to handle inscription txn generation is used instead. This is not intrusive and can be quickly removed once btcutil is upgraded.

How Has This Been Tested?

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Summary by CodeRabbit

  • New Features

    • Introduced a logging feature to enhance visibility of test executions.
    • Added a test case for extracting Bitcoin inscription memo names.
    • Implemented an Express.js server with endpoints for Bitcoin transaction handling.
    • Developed a ScriptBuilder for constructing tapscripts used in Bitcoin transactions.
    • Created a utility function for formatting public keys in Bitcoin transactions.
  • Bug Fixes

    • Improved transaction processing by updating methods to include witness data for better validation.
  • Documentation

    • Added configuration files like tsconfig.json for TypeScript setup.
  • Chores

    • Updated Docker configurations to include a new Bitcoin sidecar service in the setup.

Co-authored-by: Lucas Bertrand <[email protected]>
@lumtis
Copy link
Member

lumtis commented Aug 21, 2024

It seems it breaks the deposit BTC test: https://github.com/zeta-chain/node/actions/runs/10471358149/job/28998509065

Is the change supposed to be backward compatible

@bitSmiley
Copy link
Contributor Author

@lumtis No, it should be backward compatible. There is an error fixed in b341832, which if OP_RETURN and inscription both cannot find the memo, then it should just return nil not error.

@bitSmiley bitSmiley requested a review from lumtis August 21, 2024 13:36
@lumtis
Copy link
Member

lumtis commented Aug 21, 2024

@lumtis No, it should be backward compatible. There is an error fixed in b341832, which if OP_RETURN and inscription both cannot find the memo, then it should just return nil not error.

Ok, rerunning

@bitSmiley
Copy link
Contributor Author

@lumtis sorry man, I just fixed the linting and change log, please help run the pipeline again.

@lumtis lumtis marked this pull request as draft August 27, 2024 09:04
@lumtis lumtis marked this pull request as ready for review September 14, 2024 00:14
@lumtis
Copy link
Member

lumtis commented Sep 14, 2024

Opening back for develop for the next release

@lumtis
Copy link
Member

lumtis commented Oct 2, 2024

Closing in favor of #2957 as discussed on Slack

@lumtis lumtis closed this Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking:cli UPGRADE_TESTS Run make start-upgrade-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants