refactor: phase-1: reorganize source files into chains, orchestrator and zetacore packages #2150

ws4charlie · 2024-05-09T19:07:35Z

Description

This PR is just a high-level package reorg according the doc. It does not touch the contents of existing structs. To reorganize the structs, I'll create new PR to do it.

created chains package to contain bitcoin, evm and interfaces
grouped app_context and core_context as one single package context
renamed package zetabridge as zetacore and dropped usages of zeta bridge or bridge in source code.
created package orchestrator to contain existing ztacore_observerXXX files and renamed methods/variables as orchestrator accordingly.
renamed source files, so we have observer.go, signer.go in each chain package; renamed struct names as Observer and Signer accordingly. The EVM Client and BTC Client were completely dropped.
put all outbound/inbound observation logic into observer_outbound.go and observer_inbound.go respectively, leaving observer.go containing non-observation logic.
deleted errors (it contains 2 constants which were moved to the package that uses them).

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.

Checklist:

I have added unit tests that prove my fix feature works

…aces to chains; renamed source files; deleted unused files

github-actions · 2024-05-09T21:34:17Z

!!!WARNING!!!
nosec detected in the following files: zetaclient/chains/bitcoin/observer/inbound.go, zetaclient/chains/bitcoin/observer/observer.go, zetaclient/chains/bitcoin/observer/outbound.go, zetaclient/chains/evm/observer/inbound.go, zetaclient/chains/evm/observer/observer.go, zetaclient/chains/bitcoin/fee.go, zetaclient/chains/bitcoin/fee_test.go, zetaclient/chains/bitcoin/observer/inbound_test.go, zetaclient/chains/bitcoin/signer/signer.go, zetaclient/chains/bitcoin/utils.go, zetaclient/chains/evm/observer/observer_test.go, zetaclient/chains/evm/signer/signer.go, zetaclient/orchestrator/orchestrator.go, zetaclient/orchestrator/orchestrator_test.go, zetaclient/zetacore/broadcast.go, zetaclient/zetacore/tx.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.

codecov · 2024-05-09T21:42:51Z

Codecov Report

Attention: Patch coverage is 31.79684% with 1598 lines in your changes are missing coverage. Please review.

Project coverage is 72.34%. Comparing base (9394bbb) to head (3fe8e0f).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2150   +/-   ##
========================================
  Coverage    72.34%   72.34%           
========================================
  Files          250      249    -1     
  Lines        14046    14007   -39     
========================================
- Hits         10162    10134   -28     
+ Misses        3423     3412   -11     
  Partials       461      461

Files	Coverage Δ
zetaclient/chains/bitcoin/fee.go	`49.49% <ø> (ø)`
zetaclient/chains/bitcoin/tx_script.go	`93.10% <ø> (ø)`
zetaclient/chains/bitcoin/utils.go	`26.92% <ø> (ø)`
zetaclient/chains/evm/signer/outbound_tx_data.go	`69.33% <100.00%> (ø)`
zetaclient/chains/evm/validation.go	`100.00% <ø> (ø)`
zetaclient/config/types.go	`0.00% <ø> (ø)`
zetaclient/supplychecker/logger.go	`60.00% <ø> (ø)`
zetaclient/context/zetacore_context.go	`86.29% <92.85%> (ø)`
zetaclient/context/app_context.go	`0.00% <0.00%> (ø)`
zetaclient/keys/keys.go	`45.74% <0.00%> (ø)`
... and 16 more

lumtis

From the doc:

I think we should rename Client to Observer for the logic managing watching inbounds and outbounds, since the Signer can also be considered as a client to would clarify differences. Any opposition against this?
I think we can use two dsub packages fr signer and observer for each chains

changelog.md

cmd/zetaclientd/debug.go

zetaclient/testutils/mocks/cometbft_client.go

…ore-package

ws4charlie · 2024-05-10T18:53:08Z

From the doc:

I think we should rename Client to Observer for the logic managing watching inbounds and outbounds, since the Signer can also be considered as a client to would clarify differences. Any opposition against this?

I think we can use two dsub packages fr signer and observer for each chains

We've been on the same page. I just included the renaming part right in this same PR in commit 6a2a68a . A new Client that contains both Observer and Signer will be introduced in upcoming PR.

lumtis

This looks good to me.
Just one thing: I think we can rename CoreClient into ZetacoreClient
CoreClient is a bit unclear

…ore-package

ws4charlie · 2024-05-10T22:47:17Z

This looks good to me. Just one thing: I think we can rename CoreClient into ZetacoreClient CoreClient is a bit unclear

Now variables are unified as zetacoreClient. Also, the single word zetacore is now used as a unified terminology.

ws4charlie · 2024-05-10T22:48:58Z

From the doc:

I think we should rename Client to Observer for the logic managing watching inbounds and outbounds, since the Signer can also be considered as a client to would clarify differences. Any opposition against this?

I think we can use two dsub packages fr signer and observer for each chains

The sub packages signer and observer were created by this commit f8e9ccb

skosito

lgtm, it seems some files are without tests, but not sure if those are added now or moved, so just to make sure we are tracking missing coverage for those

zetaclient/chains/bitcoin/observer/inbound.go

cmd/zetaclientd/utils.go

zetaclient/chains/bitcoin/observer/observer.go

zetaclient/context/app_context.go

…ore-package

ws4charlie · 2024-05-16T14:24:35Z

lgtm, it seems some files are without tests, but not sure if those are added now or moved, so just to make sure we are tracking missing coverage for those

This PR didn't add things, it just renamed/reorganized things into proper packages

ws4charlie added 2 commits May 7, 2024 14:51

move evm and bitcoin to chains package

562aa03

dropped zetabridge; merged app_context and core_context; moved interf…

72c8cad

…aces to chains; renamed source files; deleted unused files

ws4charlie requested review from brewmaster012, kingpinXD, lumtis and skosito as code owners May 9, 2024 19:07

github-actions bot added the breaking:cli label May 9, 2024

resolved conflicts

1df9f60

github-actions bot added the nosec label May 9, 2024

added changelog entry

13e4c9a

fix code generation

ea72fde

lumtis reviewed May 10, 2024

View reviewed changes

ws4charlie and others added 2 commits May 10, 2024 13:45

dropped EVM Client and BTC Client; Used Observer instead

6a2a68a

Merge branch 'develop' into refactor-create-chains-orchestrator-zetac…

9f2c805

…ore-package

lumtis reviewed May 10, 2024

View reviewed changes

ws4charlie and others added 3 commits May 10, 2024 17:37

created sub-package signer and observer for each chain

f8e9ccb

use explicit name ; use single word

973f378

Merge branch 'develop' into refactor-create-chains-orchestrator-zetac…

e0aaac8

…ore-package

unified terminology as single-word

1def779

skosito approved these changes May 13, 2024

View reviewed changes

zetaclient/chains/bitcoin/observer/inbound.go Show resolved Hide resolved

cmd/zetaclientd/utils.go Outdated Show resolved Hide resolved

zetaclient/chains/bitcoin/observer/observer.go Show resolved Hide resolved

zetaclient/context/app_context.go Outdated Show resolved Hide resolved

lumtis approved these changes May 14, 2024

View reviewed changes

ws4charlie and others added 2 commits May 16, 2024 08:47

Merge branch 'develop' into refactor-create-chains-orchestrator-zetac…

169d3ee

…ore-package

using single word for ZetaCoreContext

3fe8e0f

ws4charlie merged commit d9a5d44 into develop May 16, 2024
20 of 21 checks passed

ws4charlie deleted the refactor-create-chains-orchestrator-zetacore-package branch May 16, 2024 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: phase-1: reorganize source files into chains, orchestrator and zetacore packages #2150

refactor: phase-1: reorganize source files into chains, orchestrator and zetacore packages #2150

ws4charlie commented May 9, 2024 •

edited

Loading

github-actions bot commented May 9, 2024 •

edited

Loading

codecov bot commented May 9, 2024 •

edited

Loading

lumtis left a comment

ws4charlie commented May 10, 2024 •

edited

Loading

lumtis left a comment

ws4charlie commented May 10, 2024

ws4charlie commented May 10, 2024

skosito left a comment

ws4charlie commented May 16, 2024

refactor: phase-1: reorganize source files into chains, orchestrator and zetacore packages #2150

refactor: phase-1: reorganize source files into chains, orchestrator and zetacore packages #2150

Conversation

ws4charlie commented May 9, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

github-actions bot commented May 9, 2024 • edited Loading

codecov bot commented May 9, 2024 • edited Loading

Codecov Report

lumtis left a comment

Choose a reason for hiding this comment

ws4charlie commented May 10, 2024 • edited Loading

lumtis left a comment

Choose a reason for hiding this comment

ws4charlie commented May 10, 2024

ws4charlie commented May 10, 2024

skosito left a comment

Choose a reason for hiding this comment

ws4charlie commented May 16, 2024

ws4charlie commented May 9, 2024 •

edited

Loading

github-actions bot commented May 9, 2024 •

edited

Loading

codecov bot commented May 9, 2024 •

edited

Loading

ws4charlie commented May 10, 2024 •

edited

Loading