Skip to content

Conversation

@yihuang
Copy link

@yihuang yihuang commented Sep 22, 2023

Description

WIP: #355

Solution:

  • to decouple sender address verification from execution, we must first enforce user setting the From field in the msg
  • change From field from string to bytes, avoid heavy checksum hex encoding of go-ethereum
  • also minimize checksum hex encoding in other places in state machine.

Benchmark Results

Run the BenchmarkERC20Transfer in cronos repo.

goos: darwin
goarch: amd64
pkg: github.com/crypto-org-chain/cronos/v2/app
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
                         │ /tmp/after.txt │           /tmp/after2.txt           │
                         │     sec/op     │    sec/op     vs base               │
ERC20Transfer/memdb-12       612.9m ± 19%   602.1m ± 12%        ~ (p=0.818 n=6)
ERC20Transfer/leveldb-12     700.5m ± 18%   593.4m ±  1%  -15.28% (p=0.002 n=6)
ERC20Transfer/memiavl-12     604.1m ± 28%   582.6m ± 23%        ~ (p=0.240 n=6)
geomean                      637.7m         592.6m         -7.07%

                         │ /tmp/after.txt │          /tmp/after2.txt           │
                         │      B/op      │     B/op      vs base              │
ERC20Transfer/memdb-12       176.4Mi ± 0%   174.1Mi ± 0%  -1.32% (p=0.002 n=6)
ERC20Transfer/leveldb-12     178.2Mi ± 0%   175.9Mi ± 0%  -1.30% (p=0.002 n=6)
ERC20Transfer/memiavl-12     171.0Mi ± 0%   168.7Mi ± 0%  -1.36% (p=0.002 n=6)
geomean                      175.2Mi        172.9Mi       -1.33%

                         │ /tmp/after.txt │          /tmp/after2.txt          │
                         │   allocs/op    │  allocs/op   vs base              │
ERC20Transfer/memdb-12        2.802M ± 0%   2.776M ± 0%  -0.93% (p=0.002 n=6)
ERC20Transfer/leveldb-12      2.801M ± 0%   2.775M ± 0%  -0.93% (p=0.002 n=6)
ERC20Transfer/memiavl-12      2.747M ± 0%   2.721M ± 0%  -0.95% (p=0.002 n=6)
geomean                       2.783M        2.757M       -0.93%

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)

WIP: #355

Solution:
- to decouple sender address verification from execution, we must first enforce user setting the From field in the msg
@yihuang yihuang requested a review from mmsqe September 22, 2023 04:34
sdk.NewAttribute(sdk.AttributeKeySender, sender),
sdk.NewAttribute(types.AttributeKeyTxType, fmt.Sprintf("%d", tx.Type())),
sdk.NewAttribute(sdk.AttributeKeySender, types.HexAddress(msg.From)),
sdk.NewAttribute(types.AttributeKeyTxType, strconv.Itoa(int(tx.Type()))),

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion

Potential integer overflow by integer type conversion
mmsqe
mmsqe previously approved these changes Sep 22, 2023
HuangYi added 2 commits September 22, 2023 15:50
@yihuang yihuang enabled auto-merge (squash) September 22, 2023 15:23
@yihuang yihuang requested a review from mmsqe September 22, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants