Skip to content

miner: set tx index logs#2983

Merged
fjl merged 1 commit intoethereum:developfrom
bas-vk:txindex
Sep 14, 2016
Merged

miner: set tx index logs#2983
fjl merged 1 commit intoethereum:developfrom
bas-vk:txindex

Conversation

@bas-vk
Copy link
Copy Markdown
Member

@bas-vk bas-vk commented Sep 8, 2016

The miner creates a new state record for each transaction. One of the parameters for these records is the transaction index. Currently this index is hard coded to zero and causes all logs to have a transaction index of 0. This PR corrects this.

@robotally
Copy link
Copy Markdown

robotally commented Sep 8, 2016

Vote Count Reviewers
👍 2 @fjl @karalabe
👎 0

Updated: Mon Sep 12 10:28:43 UTC 2016

@mention-bot
Copy link
Copy Markdown

@bas-vk, thanks for your PR! By analyzing the annotation information on this pull request, we identified @karalabe, @fjl and @obscuren to be potential reviewers

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Sep 9, 2016

👍

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Sep 9, 2016

This needs two reviews.

Comment thread miner/worker.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This i++ won't be good here. There are a few scenarios below when a transaction is discarded (gas price too low, doesn't fit into block, etc). These scenarios will all "continue looping" without processing the transaction, but the i++ will increment nonetheless, producing gaps.

Shouldn't we use env.tcount++ (incremented in L604) to keep track of the index of the currently executing transaction and completely drop i?

@bas-vk
Copy link
Copy Markdown
Member Author

bas-vk commented Sep 12, 2016

@karalabe, you are right.
Replaced it with env.tcount.

@karalabe
Copy link
Copy Markdown
Member

LGTM 👍

@fjl PTAL that it makes sense with the fix.

Copy link
Copy Markdown
Contributor

@fjl fjl left a comment

Choose a reason for hiding this comment

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

good catch @karalabe, LGTM

@fjl fjl merged commit 0f6f83a into ethereum:develop Sep 14, 2016
@fjl fjl removed the review label Sep 14, 2016
@fjl fjl added this to the 1.5.0 milestone Sep 16, 2016
srene pushed a commit to dymensionxyz/go-ethereum that referenced this pull request Mar 26, 2025
Merge branch 'tag v1.5.8' into develop
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.

6 participants