feat: Bitcoin block header and merkle proof #1263

ws4charlie · 2023-10-10T04:41:33Z

Description

Added Bitcoin block header support in zetacore.
Added support for unpriviledged AddOutTxTracker for observed Bitcoin outTx.
Added a RPC to check Bitcoin merkle proofs.
Added a smoketest to test the Bitcoin merkle proof generation and use the prove RPC to verify proof validity

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

Checklist:

I have added unit tests that prove my fix feature works

…coin-header-N-proof

github-actions · 2023-10-10T04:42:16Z

!!!WARNING!!!
nosec detected in the following files: common/utils.go, common/headers.go, common/proof.go, zetaclient/bitcoin_client.go, zetaclient/btc_signer.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

…n/node into bitcoin-header-N-proof

common/headers_test.go

common/proof_test.go

contrib/localnet/orchestrator/smoketest/test_bitcoin.go

x/crosschain/keeper/keeper_out_tx_tracker.go

x/observer/keeper/grpc_query_prove.go

lumtis · 2023-10-10T18:55:29Z

x/observer/keeper/grpc_query_prove.go

-		err = txx.UnmarshalBinary(val)
-		if err != nil {
-			return nil, status.Error(codes.Internal, fmt.Sprintf("failed to unmarshal transaction: %s", err))
+		if common.IsEVMChain(req.ChainId) {


I think the current Ethereum proof system is for Ethereum specifically and not EVM, and we should use IsEthereumChain instead

Maybe @brewmaster012 can confirm

Same comment for the other places where we use IsEVMChain

Good question. Current code base only accepts Ethereum block headers at ValidateBasic
We should explicitly restrict which chains are enabled in the source code in a consistent manner (on messages of MsgAddBlockHeader, QueryProveRequest) and MsgAddToOutTxTracker`)
@brewmaster012

We should explicitly restrict which chains are enabled in the source code in a consistent manner (on messages of MsgAddBlockHeader, QueryProveRequest) and MsgAddToOutTxTracker`)

I meant it would solve it if we replace IsEVMChain with IsEthereumChain in the different locations, isn't it?

Current Ethereum proof is based and tested on Ethereum only, so most relaible on Ethereum.
Most likely they also work for other EVM chains, if they are not messed up too much (deviating from go-ethereum too much). Klaytn might have problems, Polygon is unknown, BSC might be ok.

For now I think IsEVMChain() is fine because it basically means Ethereum/BSC.

lumtis · 2023-10-10T18:59:38Z

zetaclient/bitcoin_client.go

 }

 const (
 	minConfirmations = 0
 	maxHeightDiff    = 10000
-	dustOffset       = 2000
+	btcBlocksPerDay  = 144


Is it the same value for testnet?

I think there is no defined rule on dustOffset for neither of them (testnet, mainnet). The block time is 10 mins on average in mainnet and very variable in testnet3 (1sec to 30 mins). Looking back to this variable, the LRU cache probably can have any reasonable length (e.g. 10, 100, 500, 1000) to avoid unnecessary RPC queries as long as it won't take too much resources and impact performance. I just put a meaningful name btcBlocksPerDay be a bit clear for people.

So having btcBlocksPerDay = 144 would not be an issue when using ZetaClient with Bitcoin testnet?

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

…n/node into bitcoin-header-N-proof

lumtis

Looking good with the smoke tests
We'll have to do excperimentation

common/proof.go

x/crosschain/keeper/keeper_out_tx_tracker.go

brewmaster012

Excellent PR!
Looking forward to its merge and deployment and the stuff it enabled us to do.

…n/node into bitcoin-header-N-proof

#1292) * refactor: change default mempool version in config (#1238) * fix(`MsgWhitelistERC20`): set unique index for generate cctx (#1245) * update index * remove deprecated functions * make generate * add return value in message * initialize test * whitelist tests * make generate * fix(`observer`): remove error return in `IsAuthorized` (#1250) * update function * regenerate interfaces * update for crosschain * fix(`GetForeignCoinFromAsset`): Ethereum comparaison checksum/non-checksum format (#1261) * fix error message * compare with ETH address type * tests * goimport --------- Co-authored-by: brewmaster012 <88689859+brewmaster012@users.noreply.github.com> * feat(`fungible`): add ability to update gas limit (#1260) * add new field * update message type * message new logic * fix: Blame index update (#1264) * initial commit * added queries and unit tests * added cli * fix parse error * fix parse error 2 * fix lint and test errors * ran make generate * update index for keygen * refactor query name * refactor key calculation * refactor lib name * fix: feed sataoshi/B to zetacore and check actual outTx size (#1243) * feed sataoshi/B to zetacore and check size limit * removed fee cap * replaced magic number 1000 with constant bytesPerKB * put lowerbound, upperbound limit on sizeLimit * use actual txSize for fee calculation --------- Co-authored-by: charliec <charliec@zetachain.com> * fix: cherry pick all hotfix from v10.0.x (zero-amount, precision, etc.) (#1235) * cherry pick all hotfix from v10.0.x * adjusted code to for nosec * adusted error handling and code comments according to PR review feedback * cherry pick hotfix for bitcoin outbound performance and updated some log prints * cherry pick mock mainnet hotfix for duplicate payment on nonce 0 --------- Co-authored-by: charliec <charliec@zetachain.com> * fix: register emissions grpc server (#1257) * feat: Bitcoin block header and merkle proof (#1263) * initiated bitcoin header and proof * added smoke test for bitcoin merkle proof and RPC query * make generate * fix gosec and unit test * Update common/headers_test.go Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * code adjustment according to feedback of PR review * corrected a typo and added more comment to function * fix gosec error --------- Co-authored-by: charliec <charliec@zetachain.com> Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: brewmaster012 <88689859+brewmaster012@users.noreply.github.com> * fix: read gas limit from smart contract (#1277) * read gas limit from smart contract * add more checks for gas limit * fix(`fungible`): add CLI command to query system contract (#1252) * fix proto * fix filename * add cli query * fix(`cmd`): add notice when using `--ledger` with Ethereum HD path (#1285) * change comment * add notice for ledger * merge develop into inbound-tracker and unified proof verification * fixed gosec errors --------- Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: brewmaster012 <88689859+brewmaster012@users.noreply.github.com> Co-authored-by: kevinssgh <79858682+kevinssgh@users.noreply.github.com> Co-authored-by: charliec <charliec@zetachain.com> Co-authored-by: Tanmay <tanmay@zetachain.com>

ws4charlie added 3 commits October 1, 2023 23:40

initiated bitcoin header and proof

ff3e088

Merge branch 'develop' of https://github.com/zeta-chain/node into bit…

5bf4e88

…coin-header-N-proof

added smoke test for bitcoin merkle proof and RPC query

11c60bc

ws4charlie requested review from brewmaster012, kingpinXD, kevinssgh, fadeev and lumtis as code owners October 10, 2023 04:41

Merge branch 'develop' into bitcoin-header-N-proof

b5aa668

github-actions bot added the breaking:proto label Oct 10, 2023

github-actions bot added the nosec label Oct 10, 2023

ws4charlie added 3 commits October 9, 2023 23:49

make generate

b7c9a39

Merge branch 'bitcoin-header-N-proof' of https://github.com/zeta-chai…

da80764

…n/node into bitcoin-header-N-proof

fix gosec and unit test

be099dc

ws4charlie mentioned this pull request Oct 10, 2023

bitcoin header/proof #1221

Closed

lumtis reviewed Oct 10, 2023

View reviewed changes

ws4charlie and others added 4 commits October 10, 2023 15:49

Update common/headers_test.go

98cca27

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

Merge branch 'develop' into bitcoin-header-N-proof

d898787

code adjustment according to feedback of PR review

5a193e8

Merge branch 'bitcoin-header-N-proof' of https://github.com/zeta-chai…

dfe9eab

…n/node into bitcoin-header-N-proof

lumtis approved these changes Oct 11, 2023

View reviewed changes

sync with develop and fixed gosec errors

cd0c3e9

brewmaster012 reviewed Oct 12, 2023

View reviewed changes

common/proof.go Show resolved Hide resolved

brewmaster012 reviewed Oct 12, 2023

View reviewed changes

x/crosschain/keeper/keeper_out_tx_tracker.go Outdated Show resolved Hide resolved

brewmaster012 approved these changes Oct 12, 2023

View reviewed changes

brewmaster012 and others added 4 commits October 12, 2023 09:27

Merge branch 'develop' into bitcoin-header-N-proof

c98c5c8

corrected a typo and added more comment to function

9ce0e40

Merge branch 'bitcoin-header-N-proof' of https://github.com/zeta-chai…

bb8de02

…n/node into bitcoin-header-N-proof

fix gosec error

7cdc29a

ws4charlie merged commit b6bfa57 into develop Oct 12, 2023
11 checks passed

lumtis linked an issue Oct 12, 2023 that may be closed by this pull request

bitcoin header/proof #1221

Closed

lumtis deleted the bitcoin-header-N-proof branch October 27, 2023 23:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Bitcoin block header and merkle proof #1263

feat: Bitcoin block header and merkle proof #1263

ws4charlie commented Oct 10, 2023

github-actions bot commented Oct 10, 2023 •

edited

lumtis Oct 10, 2023

ws4charlie Oct 10, 2023 •

edited

lumtis Oct 10, 2023

brewmaster012 Oct 12, 2023 •

edited

lumtis Oct 10, 2023

ws4charlie Oct 10, 2023

lumtis Oct 11, 2023

lumtis left a comment

brewmaster012 left a comment

feat: Bitcoin block header and merkle proof #1263

feat: Bitcoin block header and merkle proof #1263

Conversation

ws4charlie commented Oct 10, 2023

Description

Type of change

How Has This Been Tested?

Checklist:

github-actions bot commented Oct 10, 2023 • edited

lumtis Oct 10, 2023

Choose a reason for hiding this comment

ws4charlie Oct 10, 2023 • edited

Choose a reason for hiding this comment

lumtis Oct 10, 2023

Choose a reason for hiding this comment

brewmaster012 Oct 12, 2023 • edited

Choose a reason for hiding this comment

lumtis Oct 10, 2023

Choose a reason for hiding this comment

ws4charlie Oct 10, 2023

Choose a reason for hiding this comment

lumtis Oct 11, 2023

Choose a reason for hiding this comment

lumtis left a comment

Choose a reason for hiding this comment

brewmaster012 left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2023 •

edited

ws4charlie Oct 10, 2023 •

edited

brewmaster012 Oct 12, 2023 •

edited