Skip to content

les: fix TxStatusMsg RLP coding#15974

Merged
karalabe merged 1 commit into
ethereum:masterfrom
zsfelfoldi:tx-status-fix2
Jan 26, 2018
Merged

les: fix TxStatusMsg RLP coding#15974
karalabe merged 1 commit into
ethereum:masterfrom
zsfelfoldi:tx-status-fix2

Conversation

@zsfelfoldi
Copy link
Copy Markdown
Contributor

Fixes #15943
Replaces #15956

Comment thread les/handler.go
var resp struct {
ReqID, BV uint64
Status []core.TxStatus
Status []txStatus
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 is an odd one. Why change this? Is this a network protocol packet? If so, don't we have it declared already in protocol.go. If not, perhaps we should? Just wondering here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is a protocol packet and it is declared in protocol.go. Before the cleanup there was no les.txStatus, just core.TxStatus which contained the position and error fields too. When les.txStatus was created, we forgot to replace this instance (another reason the whole thing wasn't working).

@zsfelfoldi zsfelfoldi added this to the 1.8.0 milestone Jan 25, 2018
Copy link
Copy Markdown
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM

@karalabe karalabe merged commit 23bca0f into ethereum:master Jan 26, 2018
prestonvanloon pushed a commit to prestonvanloon/go-ethereum that referenced this pull request Apr 2, 2018
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
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