Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Problem: tx log attribtue value not parsable by some client #615

Merged
merged 3 commits into from
Oct 5, 2021
Merged

Problem: tx log attribtue value not parsable by some client #615

merged 3 commits into from
Oct 5, 2021

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Oct 3, 2021 •
edited
Loading

Closes: #614

Solution:

  • encode the value to json string rather than bytes

Description

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

yihuang
yihuang commented Oct 3, 2021
View reviewed changes
rpc/ethereum/backend/utils.go Outdated Show resolved Hide resolved
yihuang
yihuang commented Oct 3, 2021
View reviewed changes
x/evm/keeper/msg_server.go Outdated Show resolved Hide resolved
@yihuang
Problem: tx log attribtue value not parsable by some client
c53ebc8 
Closes: evmos#614

Solution:
- encode the value to json string rather than bytes

Apply suggestions from code review
@codecov
Copy link

codecov bot commented Oct 3, 2021 •
edited
Loading

Codecov Report

Merging #615 (317c2f7) into main (d6423f0) will decrease coverage by 0.03%.
The diff coverage is 40.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
- Coverage   51.80%   51.76%   -0.04%     
==========================================
  Files          65       65              
  Lines        5459     5461       +2     
==========================================
- Hits         2828     2827       -1     
- Misses       2472     2474       +2     
- Partials      159      160       +1
Impacted Files Coverage Δ
x/evm/keeper/msg_server.go 73.58% <40.00%> (-4.85%) ⬇️

Piyushblog
Piyushblog reviewed Oct 3, 2021
View reviewed changes
Copy link

@Piyushblog Piyushblog left a comment

Choose a reason for hiding this comment

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

It is good but somewhere it is wrong

@thomas-nguy
Copy link
Contributor

related to this issue informalsystems/tendermint-rs#1003

@yihuang yihuang mentioned this pull request Oct 4, 2021
Closed
13 tasks
fedekunze
fedekunze suggested changes Oct 4, 2021
View reviewed changes
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM. Only left 1 comment. Let's also add a changelog entry 👍

rpc/ethereum/backend/utils.go Outdated Show resolved Hide resolved
thomas-nguy referenced this pull request in crypto-org-chain/ethermint Oct 4, 2021
@thomas-nguy
Cherry pick tharsis#615: tx log attribtue value not parsable by some …
4d21271 
…client
yihuang added a commit to crypto-org-chain/cronos that referenced this pull request Oct 4, 2021
@yihuang
hotfix release
9996676 
cherry picked two bug fix:
- [iterator on deeply nested cache contexts is extremely slow ](evmos/ethermint#617)
- [tx log attribtue value not parsable by some client ](evmos/ethermint#615)
yihuang added a commit to crypto-org-chain/cronos that referenced this pull request Oct 4, 2021
@yihuang
Problem: two emergency bug fixes not included
f8a7892 
Solution:
- cherry picked two bug fix:
 - [iterator on deeply nested cache contexts is extremely slow ](evmos/ethermint#617)
 - [tx log attribtue value not parsable by some client ](evmos/ethermint#615)
yihuang added a commit to crypto-org-chain/cronos that referenced this pull request Oct 4, 2021
@yihuang
Problem: two emergency bug fixes not included
c94435f 
Solution:
- cherry picked two bug fix:
 - [iterator on deeply nested cache contexts is extremely slow ](evmos/ethermint#617)
 - [tx log attribtue value not parsable by some client ](evmos/ethermint#615)
@yihuang yihuang mentioned this pull request Oct 4, 2021
Merged
13 tasks
yihuang added a commit to crypto-org-chain/cronos that referenced this pull request Oct 4, 2021
@yihuang
Problem: two emergency bug fixes not included (#134)
ce17b4e 
Solution:
- cherry picked two bug fix:
 - [iterator on deeply nested cache contexts is extremely slow ](evmos/ethermint#617)
 - [tx log attribtue value not parsable by some client ](evmos/ethermint#615)
@fedekunze
Copy link
Contributor

please add a changelog entry too so that we can track the bug-fixes

@fedekunze fedekunze enabled auto-merge (squash) October 5, 2021 10:19
@fedekunze fedekunze merged commit cda968b into evmos:main Oct 5, 2021
@yihuang yihuang deleted the txlog branch October 6, 2021 01:04
fedekunze added a commit that referenced this pull request Oct 7, 2021
@yihuang @fedekunze
evm, rpc: fix tx log attribute value is not parsable by some client (#…
386943f 
…615)

* Problem: tx log attribute value not parsable by some client

Closes: #614

Solution:
- encode the value to json string rather than bytes

Apply suggestions from code review

* rm cdc and changelog

Co-authored-by: Federico Kunze Küllmer <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
fedekunze added a commit that referenced this pull request Oct 7, 2021
@fedekunze @dependabot
@danburck @yihuang @davcrypto
release: v0.7.0 cherry picks (#645)
de83c87 
* docs: v0.6.0 changelog (#605) (#606)

* docs: v0.6.0 changelog

* update codeowners

* build(deps): bump github.com/cosmos/cosmos-sdk from 0.44.0 to 0.44.1 (#610)

* build(deps): bump github.com/cosmos/cosmos-sdk from 0.44.0 to 0.44.1

Bumps [github.com/cosmos/cosmos-sdk](https:/cosmos/cosmos-sdk) from 0.44.0 to 0.44.1.
- [Release notes](https:/cosmos/cosmos-sdk/releases)
- [Changelog](https:/cosmos/cosmos-sdk/blob/v0.44.1/CHANGELOG.md)
- [Commits](cosmos/cosmos-sdk@v0.44.0...v0.44.1)

---
updated-dependencies:
- dependency-name: github.com/cosmos/cosmos-sdk
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* changelog

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Federico Kunze <[email protected]>

* cmd: use config on genaccounts (#483)

* cmd: use config on genaccounts

* update

* c++

* rpc: fix panic (#611)

* rpc: fix panic

* fix

* c++

* rpc: restructure JSON-RPC directory and rename server config (#612)

* Restructure ethermint/rpc repo structure and change import statements

* Add #400 to changelog

* fix filepath in util and json_rpc

* Move #400  to unreleased section

* evm: refactor `traceTx` (#613)

* DNM: debug traceTx

* c++

* deps: bump IBC-go (#621)

* deps: bump IBC-go

* changelog

* evm, rpc:  fix tx log attribute value is not parsable by some client (#615)

* Problem: tx log attribute value not parsable by some client

Closes: #614

Solution:
- encode the value to json string rather than bytes

Apply suggestions from code review

* rm cdc and changelog

Co-authored-by: Federico Kunze Küllmer <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>

* evm: fix iterator on deeply nested cache contexts (#617)

* Problem: iterator on deeply nested cache contexts is extremely slow

Closes: #616

Solution:
- flatten cache contexts before doing `GetTxLogsTransient`

* Update x/evm/keeper/context_stack.go

* changelog

Co-authored-by: Federico Kunze Küllmer <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>

* evm: add benchmark for deep context stack (#627)

* Problem: deep context stack efficienty is not benchmarked

Closes: #626

Solution:
- add a benchmark to demonstrate an extremely inefficiency in deep
  context stack

* Update x/evm/keeper/benchmark_test.go

* prefix storage is irrelevant

* add comment to state_transition.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* rpc: support personal apis with different keyring backends (#591)

* UPDATE Unlock keyring on start

* ADD comment

* ADD validation

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* conflicts

* changelog

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Daniel Burckhardt <[email protected]>
Co-authored-by: yihuang <[email protected]>
Co-authored-by: davcrypto <[email protected]>
@coderabbitai coderabbitai bot mentioned this pull request Apr 19, 2024
Merged
13 tasks
This was referenced Apr 26, 2024
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@thomas-nguy thomas-nguy thomas-nguy approved these changes

@Piyushblog Piyushblog Piyushblog left review comments

@fedekunze fedekunze fedekunze approved these changes

@jolube jolube Awaiting requested review from jolube

@khoslaventures khoslaventures Awaiting requested review from khoslaventures

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

Successfully merging this pull request may close these issues.

Problem: tx log attribute value is not parsable by some clients
4 participants
@yihuang @thomas-nguy @fedekunze @Piyushblog