Skip to content
This repository was archived by the owner on May 22, 2023. It is now read-only.

Log transaction hash as %s instead of %x#803

Merged
beaurancourt merged 7 commits intomasterfrom
transaction-hash-log
Jun 21, 2021
Merged

Log transaction hash as %s instead of %x#803
beaurancourt merged 7 commits intomasterfrom
transaction-hash-log

Conversation

@nkuba
Copy link
Copy Markdown
Member

@nkuba nkuba commented Jun 1, 2021

The Hash type has a dedicated function to convert the value to string.
It is converted to hex with the 0x prefix, which seems more consistent for ethereum.

In keep-common code generation we generate the loggers for transaction id with Hash().Hex() function which produces 0x prefixed values. (see: https://github.com/keep-network/keep-common/blob/0d4f5d5ec604ac41aa84b9ef8078038d7592fc43/tools/generators/ethlike/contract_non_const_methods.go.tmpl#L72)

With this PR we will be more consistent across the logs.

@nkuba nkuba requested a review from a team June 1, 2021 14:33
nkuba added a commit to keep-network/keep-common that referenced this pull request Jun 1, 2021
Hash type implements `String()` function that can be used to log the
value with `%s` format. It is a tiny simplification, that we introduce
for consistence with `keep-ecdsa` changes provided in keep-network/keep-ecdsa#803
@nkuba nkuba added this to the v1.8.0 milestone Jun 3, 2021
@nkuba nkuba requested a review from beaurancourt June 15, 2021 15:46
nkuba added 2 commits June 15, 2021 17:56
The `Hash` type has dedicated function to convert the value to String.
It is converted to hex with `0x` prefix, which seems more consistent for
ethereum. In keep-common code genreation we generate the loggers for
transaction id with `Hash().Hex()` function which produces `0x` prefixed
values. With this change we will be consistent.
The latest version of keep-common has two updates:
- adds space between function name and params
- logs transaction hash using %s

The code in this commit was auto-generated.
@nkuba nkuba force-pushed the transaction-hash-log branch from 4943c55 to 6442299 Compare June 15, 2021 15:56
@@ -238,7 +238,7 @@ func (becdsak *BondedECDSAKeep) DistributeERC20Reward(
) (*types.Transaction, error) {
becdsakLogger.Debug(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Was this written before we started using Debugf? Want to change it or leave it in?

Copy link
Copy Markdown

@beaurancourt beaurancourt Jun 15, 2021

Choose a reason for hiding this comment

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

oh wait this file is generated @_@

@nkuba nkuba requested a review from beaurancourt June 21, 2021 13:59
@beaurancourt beaurancourt merged commit d656568 into master Jun 21, 2021
@beaurancourt beaurancourt deleted the transaction-hash-log branch June 21, 2021 14:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants