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

fix: merge develop into inbound-tracker and unified proof verification #1292

Merged
merged 16 commits into from
Oct 13, 2023

Conversation

ws4charlie
Copy link
Contributor

@ws4charlie ws4charlie commented Oct 13, 2023

Description

  1. Merge develop into inbound-tracker branch.
  2. Unified proof verification logic for both EVM and Bitcoin proof in outTx tracker and restored some old authorization logic.
  3. Unified proof verfication logic in inTx tracker.
  4. Fixed gosec errors.

Closes:

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.

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

Checklist:

  • I have added unit tests that prove my fix feature works

lumtis and others added 14 commits October 10, 2023 09:12
* update index

* remove deprecated functions

* make generate

* add return value in message

* initialize test

* whitelist tests

* make generate
* update function

* regenerate interfaces

* update for crosschain
…cksum format (#1261)

* fix error message

* compare with ETH address type

* tests

* goimport

---------

Co-authored-by: brewmaster012 <[email protected]>
* add new field

* update message type

* message new logic
* 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
* 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 <[email protected]>
….) (#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 <[email protected]>
* 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 <[email protected]>

* code adjustment according to feedback of PR review

* corrected a typo and added more comment to function

* fix gosec error

---------

Co-authored-by: charliec <[email protected]>
Co-authored-by: Lucas Bertrand <[email protected]>
Co-authored-by: brewmaster012 <[email protected]>
* read gas limit from smart contract

* add more checks for gas limit
* fix proto

* fix filename

* add cli query
@github-actions
Copy link

github-actions bot commented Oct 13, 2023

!!!WARNING!!!
nosec detected in the following files: common/utils.go, common/coin.go, common/headers.go, common/proof.go, x/crosschain/client/querytests/in_tx_tracker.go, x/crosschain/keeper/cctx_utils.go, x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go, zetaclient/bitcoin_client.go, zetaclient/btc_signer.go, zetaclient/evm_client.go, zetaclient/inbound_tracker.go, zetaclient/zetacore_observer.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.

@github-actions github-actions bot added the nosec label Oct 13, 2023
if strings.EqualFold(hash.TxHash, msg.TxHash) {
isDup = true
if isProven {
hash.Proved = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this value get modified in the tracker? If not then the variable has no use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hash.Proved value is used nowhere for now. Moving forward, we'll gradually utilize the Proved flag in a way that whenever a tracker hash gets its Proved flag set to true, it will be treated as a true hash of outTx/inTx and be trusted by any party.

@ws4charlie ws4charlie merged commit 1ab1338 into inbound-tracker Oct 13, 2023
@lumtis lumtis deleted the inbound-tracker-code-unification branch October 27, 2023 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants