8000 add check logic on delegate tx by JayChoi1736 · Pull Request #1372 · klaytn/klaytn · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Aug 19, 2024. It is now read-only.

add check logic on delegate tx #1372

Merged
merged 17 commits into from
Jun 28, 2022
Merged

add check logic on delegate tx #1372

merged 17 commits into from
Jun 28, 2022

Conversation

JayChoi1736
Copy link
Contributor
@JayChoi1736 JayChoi1736 commented May 13, 2022 •
edited
Loading

Proposed changes

  • add check logic on txpool when sender = feepayer

Types of changes

Please put an x in the boxes related to your change.

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Related issues

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@JayChoi1736 JayChoi1736 requested review from mckim19 and yoomee1313 May 13, 2022 01:22
hqjang-pepper
hqjang-pepper previously approved these changes May 13, 2022
View reviewed changes
@JayChoi1736 JayChoi1736 self-assigned this May 13, 2022
aidan-kwon
aidan-kwon reviewed May 13, 2022
View reviewed changes
Copy link
Member
@aidan-kwon aidan-kwon left a comment

Choose a reason for hiding this comment

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

Great!
Could you consider adding the same validation in

klaytn/blockchain/tx_list.go

Line 313 in 9f2f3f9

func (l *txList) Filter(senderBalance *big.Int, pool *TxPool) (types.Transactions, types.Transactions) {
?
The balance shoulde be checked affter the txpool additon. With each block insertion, txpool examine all transactions in pending/queue txs again. Check

klaytn/blockchain/tx_pool.go

Line 362 in 9f2f3f9

func (pool *TxPool) reset(oldHead, newHead *types.Header) {

@JayChoi1736 JayChoi1736 requested a review from aidan-kwon May 20, 2022 02:51
yoomee1313
yoomee1313 reviewed May 23, 2022
View reviewed changes
@JayChoi1736
Update comment on check logic
88da93a 
Co-authored-by: yumiel yoomee1313 <yumiel.ko@krustuniverse.com>
@JayChoi1736 JayChoi1736 requested a review from yoomee1313 May 24, 2022 05:00
< 8000 div class="js-timeline-item js-timeline-progressive-focus-container" data-gid="PRR_kwDOC2aNa846kYDS">
ehnuje
ehnuje reviewed May 24, 2022
View reviewed changes
Copy link
Contributor
@ehnuje ehnuje left a comment

Choose a reason for hiding this comment

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

How about adding a test case for your change?

kjhman21
kjhman21 reviewed May 29, 2022
View reviewed changes
@JayChoi1736 JayChoi1736 reopened this May 31, 2022
@JayChoi1736
Copy link
Contributor Author

How about adding a test case for your change?

added on tx_pool_test. thx!

kjhman21
kjhman21 reviewed Jun 13, 2022
View reviewed changes
kjhman21
kjhman21 reviewed Jun 13, 2022
View reviewed changes
@JayChoi1736 JayChoi1736 requested a review from kjhman21 June 14, 2022 07:38
kjhman21
kjhman21 reviewed Jun 15, 2022
View reviewed changes
kjhman21
kjhman21 reviewed Jun 15, 2022
View reviewed changes
JayChoi1736 and others added 3 commits June 16, 2022 08:47
@JayChoi1736
add comment
52eef73 
Co-authored-by: Junghyun Colin Kim <colin.klaytn@krustuniverse.com>
@JayChoi1736
minor change to reduce confusion
29590ec 
Co-authored-by: Junghyun Colin Kim <colin.klaytn@krustuniverse.com>
@JayChoi1736
Merge branch 'klaytn:dev' into delegate_pending
98f2272
kjhman21
kjhman21 approved these changes Jun 16, 2022
View reviewed changes
Copy link
@kjhman21 kjhman21 left a comment

Choose a reason for hiding this comment

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

LGTM.

@JayChoi1736 JayChoi1736 merged commit a76bff0 into klaytn:dev Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kjhman21 kjhman21 kjhman21 approved these changes

@aidan-kwon aidan-kwon aidan-kwon approved these changes

@yoomee1313 yoomee1313 yoomee1313 approved these changes

@KimKyungup KimKyungup Awaiting requested review from KimKyungup

@jeongkyun-oh jeongkyun-oh Awaiting requested review from jeongkyun-oh

@mckim19 mckim19 Awaiting requested review from mckim19

@aeharvlee aeharvlee Awaiting requested review from aeharvlee

6D47 @jimni1222 jimni1222 Awaiting requested review from jimni1222

@kjeom kjeom Awaiting requested review from kjeom

@sirano11 sirano11 Awaiting requested review from sirano11

@hqjang-pepper hqjang-pepper Awaiting requested review from hqjang-pepper

@ehnuje ehnuje Awaiting requested review from ehnuje

Assignees

@JayChoi1736 JayChoi1736

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@JayChoi1736 @ehnuje @kjeom @kjhman21 @hqjang-pepper @aidan-kwon @yoomee1313
0