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(zetaclient): add support for EIP-1559 outbound transactions #2387

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* [2339](https://github.com/zeta-chain/node/pull/2339) - add binaries related question to syncing issue form
* [2366](https://github.com/zeta-chain/node/pull/2366) - add migration script for adding authorizations table
* [2372](https://github.com/zeta-chain/node/pull/2372) - add queries for tss fund migration info
* [2387](https://github.com/zeta-chain/node/pull/2387) - add support for EIP-1559 outbound transactions

### Refactor

Expand Down
4 changes: 2 additions & 2 deletions cmd/zetaclientd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func start(_ *cobra.Command, _ []string) error {
}
}

masterLogger := logger.Std
masterLogger := logger.Logger
startLogger := masterLogger.With().Str("module", "startup").Logger()

// Wait until zetacore is up
Expand Down Expand Up @@ -299,7 +299,7 @@ func start(_ *cobra.Command, _ []string) error {
}

// Orchestrator wraps the zetacore client and adds the observers and signer maps to it . This is the high level object used for CCTX interactions
orchestrator := orchestrator.NewOrchestrator(zetacoreClient, signerMap, observerMap, masterLogger, telemetryServer)
orchestrator := orchestrator.New(zetacoreClient, signerMap, observerMap, masterLogger, telemetryServer)
err = orchestrator.MonitorCore(appContext)
if err != nil {
startLogger.Error().Err(err).Msg("Orchestrator failed to start")
Expand Down
16 changes: 8 additions & 8 deletions cmd/zetaclientd/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func CreateSignerMap(
}
evmChainParams, found := zetacoreContext.GetEVMChainParams(evmConfig.Chain.ChainId)
if !found {
logger.Std.Error().Msgf("ChainParam not found for chain %s", evmConfig.Chain.String())
logger.Error().Msgf("ChainParam not found for chain %s", evmConfig.Chain.String())
continue
}
mpiAddress := ethcommon.HexToAddress(evmChainParams.ConnectorContractAddress)
Expand All @@ -92,7 +92,7 @@ func CreateSignerMap(
mpiAddress,
erc20CustodyAddress)
if err != nil {
logger.Std.Error().Err(err).Msgf("NewEVMSigner error for chain %s", evmConfig.Chain.String())
logger.Error().Err(err).Msgf("NewEVMSigner error for chain %s", evmConfig.Chain.String())
continue
}
signerMap[evmConfig.Chain.ChainId] = signer
Expand All @@ -102,7 +102,7 @@ func CreateSignerMap(
if enabled {
signer, err := btcsigner.NewSigner(btcChain, zetacoreContext, tss, ts, logger, btcConfig)
if err != nil {
logger.Std.Error().Err(err).Msgf("NewBTCSigner error for chain %s", btcChain.String())
logger.Error().Err(err).Msgf("NewBTCSigner error for chain %s", btcChain.String())
} else {
signerMap[btcChain.ChainId] = signer
}
Expand All @@ -129,14 +129,14 @@ func CreateChainObserverMap(
}
chainParams, found := zetacoreContext.GetEVMChainParams(evmConfig.Chain.ChainId)
if !found {
logger.Std.Error().Msgf("ChainParam not found for chain %s", evmConfig.Chain.String())
logger.Error().Msgf("ChainParam not found for chain %s", evmConfig.Chain.String())
continue
}

// create EVM client
evmClient, err := ethclient.Dial(evmConfig.Endpoint)
if err != nil {
logger.Std.Error().Err(err).Msgf("error dailing endpoint %s", evmConfig.Endpoint)
logger.Error().Err(err).Msgf("error dailing endpoint %s", evmConfig.Endpoint)
continue
}

Expand All @@ -153,7 +153,7 @@ func CreateChainObserverMap(
ts,
)
if err != nil {
logger.Std.Error().Err(err).Msgf("NewObserver error for evm chain %s", evmConfig.Chain.String())
logger.Error().Err(err).Msgf("NewObserver error for evm chain %s", evmConfig.Chain.String())
continue
}
observerMap[evmConfig.Chain.ChainId] = observer
Expand All @@ -170,7 +170,7 @@ func CreateChainObserverMap(
if enabled {
btcClient, err := btcrpc.NewRPCClient(btcConfig)
if err != nil {
logger.Std.Error().Err(err).Msgf("error creating rpc client for bitcoin chain %s", btcChain.String())
logger.Error().Err(err).Msgf("error creating rpc client for bitcoin chain %s", btcChain.String())
} else {
// create BTC chain observer
observer, err := btcobserver.NewObserver(
Expand All @@ -185,7 +185,7 @@ func CreateChainObserverMap(
ts,
)
if err != nil {
logger.Std.Error().Err(err).Msgf("NewObserver error for bitcoin chain %s", btcChain.String())
logger.Error().Err(err).Msgf("NewObserver error for bitcoin chain %s", btcChain.String())
} else {
observerMap[btcChain.ChainId] = observer
}
Expand Down
2 changes: 1 addition & 1 deletion docs/development/LOCAL_TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ docker exec -it zetacore0 sh
Performing a query:

```bash
zetacored q bank balances zeta1amcsn7ja3608dj74xt93pcu5guffsyu2xfdcyp
zetacored q bank balances zeta1338n99nkny4yyyklj0l4adwn8hqhdevknnvdle
```

Sending a transaction:
Expand Down
17 changes: 8 additions & 9 deletions zetaclient/chains/base/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ const (

// Logger contains the base loggers
type Logger struct {
Std zerolog.Logger
zerolog.Logger
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is there any reason we use zerolog instead of the standard library slog? Maybe we could start that conversation on a parallel discussion to eventually migrate and reduce the amount of dependencies on the core node.

Compliance zerolog.Logger
}

// DefaultLoggers creates default base loggers for tests
func DefaultLogger() Logger {
return Logger{
Std: log.Logger,
Logger: log.Logger,
Compliance: log.Logger,
}
}
Expand Down Expand Up @@ -58,9 +58,12 @@ func InitLogger(cfg config.Config) (Logger, error) {
return DefaultLogger(), err
}

var (
std zerolog.Logger
compliance zerolog.Logger
)

// create loggers based on configured level and format
var std zerolog.Logger
var compliance zerolog.Logger
switch cfg.LogFormat {
case "json":
std = zerolog.New(os.Stdout).Level(zerolog.Level(cfg.LogLevel)).With().Timestamp().Logger()
Expand All @@ -80,12 +83,8 @@ func InitLogger(cfg config.Config) (Logger, error) {
if cfg.LogSampler {
std = std.Sample(&zerolog.BasicSampler{N: 5})
}
log.Logger = std // set global logger

return Logger{
Std: std,
Compliance: compliance,
}, nil
return Logger{Logger: std, Compliance: compliance}, nil
}

// openComplianceLogFile opens the compliance log file
Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/base/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestInitLogger(t *testing.T) {
require.NoError(t, err)

// should be able to print log
logger.Std.Info().Msg("print standard log")
logger.Info().Msg("print standard log")
logger.Compliance.Info().Msg("print compliance log")
})
}
Expand Down
44 changes: 28 additions & 16 deletions zetaclient/chains/base/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@

// WithLogger attaches a new logger to the observer.
func (ob *Observer) WithLogger(logger Logger) *Observer {
chainLogger := logger.Std.With().Int64("chain", ob.chain.ChainId).Logger()
chainLogger := logger.With().Int64("chain", ob.chain.ChainId).Logger()
ob.logger = ObserverLogger{
Chain: chainLogger,
Inbound: chainLogger.With().Str("module", "inbound").Logger(),
Expand All @@ -290,27 +290,26 @@

// OpenDB open sql database in the given path.
func (ob *Observer) OpenDB(dbPath string, dbName string) error {
// create db path if not exist
if _, err := os.Stat(dbPath); os.IsNotExist(err) {
err := os.MkdirAll(dbPath, os.ModePerm)
if err != nil {
return errors.Wrapf(err, "error creating db path: %s", dbPath)
var dial gorm.Dialector

// SQLite in-mem db
if strings.Contains(dbPath, ":memory:") {
dial = sqlite.Open(dbPath)
} else {
if err := ensureDirectory(dbPath); err != nil {
return errors.Wrapf(err, "unable to ensure dbPath %q", dbPath)
}
}

// use custom dbName or chain name if not provided
if dbName == "" {
dbName = ob.chain.ChainName.String()
}
path := fmt.Sprintf("%s/%s", dbPath, dbName)
// use custom dbName or chain name if not provided
if dbName == "" {
dbName = ob.chain.ChainName.String()
}

// use memory db if specified
if strings.Contains(dbPath, ":memory:") {
path = dbPath
dial = sqlite.Open(fmt.Sprintf("%s/%s", dbPath, dbName))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Consider moving this statement to a function that standardizes how we expect the db paths.
That way we make this more reusable and deterministic.

Suggested change
dial = sqlite.Open(fmt.Sprintf("%s/%s", dbPath, dbName))
dial = sqlite.Open(buildDBPath(dbPath, dbName))

}

// open db
db, err := gorm.Open(sqlite.Open(path), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
db, err := gorm.Open(dial, &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
if err != nil {
return errors.Wrap(err, "error opening db")
}
Expand All @@ -320,11 +319,24 @@
if err != nil {
return errors.Wrap(err, "error migrating db")
}

ob.db = db

return nil
}

func ensureDirectory(path string) error {
_, err := os.Stat(path)
switch {
case os.IsNotExist(err):
return os.MkdirAll(path, os.ModePerm)
case err != nil:
return err

Check warning on line 334 in zetaclient/chains/base/observer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/base/observer.go#L333-L334

Added lines #L333 - L334 were not covered by tests
default:
return nil
}
}

// CloseDB close the database.
func (ob *Observer) CloseDB() error {
dbInst, err := ob.db.DB()
Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/base/observer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func TestOpenCloseDB(t *testing.T) {
})
t.Run("should return error on invalid db path", func(t *testing.T) {
err := ob.OpenDB("/invalid/123db", "")
require.ErrorContains(t, err, "error creating db path")
require.ErrorContains(t, err, "unable to ensure dbPath")
Copy link
Contributor

Choose a reason for hiding this comment

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

"unable to ensure dbPath" is used a few times across the codebase, consider creating a const for it and reuse whenever needed.

})
}

Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/base/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewSigner(
tss: tss,
ts: ts,
logger: Logger{
Std: logger.Std.With().Int64("chain", chain.ChainId).Str("module", "signer").Logger(),
Logger: logger.With().Int64("chain", chain.ChainId).Str("module", "signer").Logger(),
Compliance: logger.Compliance,
},
}
Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/base/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestSignerGetterAndSetter(t *testing.T) {
logger := ob.Logger()

// should be able to print log
logger.Std.Info().Msg("print standard log")
logger.Info().Msg("print standard log")
logger.Compliance.Info().Msg("print compliance log")
})
}
2 changes: 1 addition & 1 deletion zetaclient/chains/bitcoin/observer/observer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func Test_NewObserver(t *testing.T) {
logger: base.Logger{},
ts: nil,
fail: true,
message: "error creating db path",
message: "unable to ensure dbPath",
},
}

Expand Down
19 changes: 9 additions & 10 deletions zetaclient/chains/bitcoin/signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
if remainingSats < 0 {
return fmt.Errorf("remainder value is negative: %d", remainingSats)
} else if remainingSats == nonceMark {
signer.Logger().Std.Info().Msgf("adjust remainder value to avoid duplicate nonce-mark: %d", remainingSats)
signer.Logger().Info().Msgf("adjust remainder value to avoid duplicate nonce-mark: %d", remainingSats)
remainingSats--
}

Expand Down Expand Up @@ -188,7 +188,7 @@
err := observer.FetchUTXOs()
if err != nil {
signer.Logger().
Std.Error().
Error().

Check warning on line 191 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L191

Added line #L191 was not covered by tests
Err(err).
Msgf("SignWithdrawTx: FetchUTXOs error: nonce %d chain %d", nonce, chain.ChainId)
}
Expand Down Expand Up @@ -224,25 +224,24 @@
return nil, err
}
if sizeLimit < bitcoin.BtcOutboundBytesWithdrawer { // ZRC20 'withdraw' charged less fee from end user
signer.Logger().Std.Info().
signer.Logger().Info().

Check warning on line 227 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L227

Added line #L227 was not covered by tests
Msgf("sizeLimit %d is less than BtcOutboundBytesWithdrawer %d for nonce %d", sizeLimit, txSize, nonce)
}
if txSize < bitcoin.OutboundBytesMin { // outbound shouldn't be blocked a low sizeLimit
signer.Logger().Std.Warn().
signer.Logger().Warn().

Check warning on line 231 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L231

Added line #L231 was not covered by tests
Msgf("txSize %d is less than outboundBytesMin %d; use outboundBytesMin", txSize, bitcoin.OutboundBytesMin)
txSize = bitcoin.OutboundBytesMin
}
if txSize > bitcoin.OutboundBytesMax { // in case of accident
signer.Logger().Std.Warn().
signer.Logger().Warn().

Check warning on line 236 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L236

Added line #L236 was not covered by tests
Msgf("txSize %d is greater than outboundBytesMax %d; use outboundBytesMax", txSize, bitcoin.OutboundBytesMax)
txSize = bitcoin.OutboundBytesMax
}

// fee calculation
// #nosec G701 always in range (checked above)
fees := new(big.Int).Mul(big.NewInt(int64(txSize)), gasPrice)
signer.Logger().
Std.Info().
signer.Logger().Info().

Check warning on line 244 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L244

Added line #L244 was not covered by tests
Msgf("bitcoin outbound nonce %d gasPrice %s size %d fees %s consolidated %d utxos of value %v",
nonce, gasPrice.String(), txSize, fees.String(), consolidatedUtxo, consolidatedValue)

Expand Down Expand Up @@ -310,7 +309,7 @@
return err
}

signer.Logger().Std.Info().Msgf("Broadcasting BTC tx , hash %s ", hash)
signer.Logger().Info().Msgf("Broadcasting BTC tx , hash %s ", hash)

Check warning on line 312 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L312

Added line #L312 was not covered by tests
return nil
}

Expand All @@ -327,11 +326,11 @@
defer func() {
outboundProcessor.EndTryProcess(outboundID)
if err := recover(); err != nil {
signer.Logger().Std.Error().Msgf("BTC TryProcessOutbound: %s, caught panic error: %v", cctx.Index, err)
signer.Logger().Error().Msgf("BTC TryProcessOutbound: %s, caught panic error: %v", cctx.Index, err)

Check warning on line 329 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L329

Added line #L329 was not covered by tests
}
}()

logger := signer.Logger().Std.With().
logger := signer.Logger().With().

Check warning on line 333 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L333

Added line #L333 was not covered by tests
Str("OutboundID", outboundID).
Str("SendHash", cctx.Index).
Logger()
Expand Down
Loading
Loading