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.

Sign up for GitHub

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

Jump to bottom

fix: some necessary code refactor for banned address and bug fix during refactor #1593

Closed
wants to merge 13 commits into from
Closed

fix: some necessary code refactor for banned address and bug fix during refactor #1593

wants to merge 13 commits into from

Conversation

ws4charlie
Copy link
Contributor

@ws4charlie ws4charlie commented Jan 17, 2024 •
edited
Loading

Description

  1. Includes a bug fix that relates to security. Inbound tracker tx hash needs to be checked against Tss address.
  2. Some refactor work to better organize inTx vote message built/posting.
  3. fix log printing

Closes: 1592

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

@ws4charlie ws4charlie changed the title Fix: some necessary code refactor for banned address and bug fix during refactor fix: some necessary code refactor for banned address and bug fix during refactor Jan 17, 2024
lumtis
lumtis approved these changes Jan 17, 2024
View reviewed changes
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

Looks good to me if it can be effectively tested on mocknet

lumtis and others added 2 commits January 19, 2024 11:43
@lumtis @ws4charlie
refactor(hotfix): support retrying sending inbound vote with higher…
6de5bc8 
… gas limit (#1601)

* chore: v12.0.0 changelog (#1578)

* fix: skip unsupported chain parameters (#1581)

Co-authored-by: Lucas Bertrand <[email protected]>

* remove debug zetaclient1

* remove comments

* tx resend

* complete renaming

* change log level and create constants

* increase retry value

* test change gas limit for outbound

* refactor vote inbound outbound into separate files

* add monitoring capability for outbound txs

* changelog

---------

Co-authored-by: Charlie Chen <[email protected]>
@ws4charlie
txTx tracker bug fix and some necessary code refactor for banned address
07f6090
@github-actions GitHub Actions
Copy link

!!!WARNING!!!
nosec detected in the following files: zetaclient/bitcoin_client.go, zetaclient/broadcast.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 Jan 19, 2024
@lumtis
Copy link
Member

lumtis commented Jan 20, 2024

Closing in favor of #1603

@lumtis lumtis closed this Jan 20, 2024
@ws4charlie ws4charlie deleted the refactor-4-banned-addresses branch February 23, 2024 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lumtis lumtis lumtis approved these changes

@kevinssgh kevinssgh kevinssgh approved these changes

@brewmaster012 brewmaster012 Awaiting requested review from brewmaster012

@kingpinXD kingpinXD Awaiting requested review from kingpinXD

@fadeev fadeev Awaiting requested review from fadeev

Assignees
No one assigned
Labels
nosec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor inTx observation so that checking logic (banned address) fits current workflow better
3 participants
@ws4charlie @lumtis @kevinssgh