Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Service transactions not propagated when remote peers use identity config parameter #10125

Closed
adetante opened this issue Jan 3, 2019 · 0 comments · Fixed by #10128
Closed

Service transactions not propagated when remote peers use identity config parameter #10125

adetante opened this issue Jan 3, 2019 · 0 comments · Fixed by #10128
Labels
F2-bug 🐞 The client fails to follow expected behavior. M4-core ⛓ Core client code / Rust. Q2-easy 💃 Can be fixed by copy and pasting from StackOverflow.
Milestone
2.4

Comments

@adetante
Copy link

adetante commented Jan 3, 2019

  • Parity Ethereum version: master, also occurs with v2.1.7-stable.

  • Operating system: MacOS

  • Installation: built from source

  • Fully synchronized: not applicable

  • Network: private network

  • Restarted: yes

Service transactions (with zero gas price) are not propagated to other nodes when using identity configuration flag.

This is due to the accepts_service_transactionfunction, which determine if remote peer can accept service transaction based on the client_id :

https://github.com/paritytech/parity-ethereum/blob/b4f8bba8430b0f9e050cdac425b632b30f0f331c/ethcore/sync/src/chain/propagator.rs#L44-L57

With identity parameter, client_id does not start with Parity-Ethereum/v and peer is considered to not supporting service transactions.

Steps to reproduce:

  • Start a private chain
  • Use identity parameter for nodes
  • Use eth_sendRawTransaction to send a transaction to a non-mining node
  • Transaction is never propagated nor mined.
@jam10o-new jam10o-new added F2-bug 🐞 The client fails to follow expected behavior. Q2-easy 💃 Can be fixed by copy and pasting from StackOverflow. M4-core ⛓ Core client code / Rust. labels Jan 3, 2019
@jam10o-new jam10o-new added this to the 2.4 milestone Jan 3, 2019
jam10o-new added a commit that referenced this issue Jan 3, 2019
@jam10o-new
fix #10125
27594cb 
fix service transaction version detection if --identity is enabled, change test to match how --identity actually works
@jam10o-new jam10o-new mentioned this issue Jan 3, 2019
Merged
sorpaas pushed a commit that referenced this issue Jan 4, 2019
@jam10o-new @sorpaas
Identity fix (#10128)
130901e 
* fix #10125

fix service transaction version detection if --identity is enabled, change test to match how --identity actually works

* fix wrong var

* get the index of  v, not /

* idx, not idx.len()

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* change version prefix to a const

* space

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>
5chdn pushed a commit that referenced this issue Jan 4, 2019
@jam10o-new @5chdn
Identity fix (#10128)
74511f6 
* fix #10125

fix service transaction version detection if --identity is enabled, change test to match how --identity actually works

* fix wrong var

* get the index of  v, not /

* idx, not idx.len()

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* change version prefix to a const

* space

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>
5chdn pushed a commit that referenced this issue Jan 4, 2019
@jam10o-new @5chdn
Identity fix (#10128)
2d450db 
* fix #10125

fix service transaction version detection if --identity is enabled, change test to match how --identity actually works

* fix wrong var

* get the index of  v, not /

* idx, not idx.len()

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* change version prefix to a const

* space

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>
5chdn added a commit that referenced this issue Jan 7, 2019
@5chdn
stable backports v2.1.11 (#10112)
e9194b0 
* version: bump stable to v2.1.11

* HF in POA Sokol (2019-01-04) (#10077)

* HF in POA Sokol (2019-01-04)

poanetwork/poa-chain-spec#91

* Update POA Core bootnodes

* Follow-up to #10105 (#10107)

* [Hit return to continue]

* ethcore: update hardcoded headers for foundation

* ethcore: update hardcoded headers for ropsten

* ethcore: update hardcoded headers for kovan

* ethcore: use consistant formatting

* ethcore: restore spaces after colon in chain spec

* ethcore: fix bootnodes in chain specs

* ethcore: fix bootnodes in chain specs

* ethcore: enforce newline at the end of chainspecs

* Identity fix (#10128)

* fix #10125

fix service transaction version detection if --identity is enabled, change test to match how --identity actually works

* fix wrong var

* get the index of  v, not /

* idx, not idx.len()

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* change version prefix to a const

* space

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* Update pwasm-utils to 0.6.1 (#10134)

* version: mark upgrade critical on kovan
5chdn added a commit that referenced this issue Jan 9, 2019
@5chdn
beta backports v2.2.6 (#10113)
e9396e1 
* version: bump beta to v2.2.6

* Update pwasm-utils to 0.6.1 (#10134)

* version: mark upgrade critical on kovan

* Identity fix (#10128)

* fix #10125

fix service transaction version detection if --identity is enabled, change test to match how --identity actually works

* fix wrong var

* get the index of  v, not /

* idx, not idx.len()

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* Update ethcore/sync/src/chain/propagator.rs

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* change version prefix to a const

* space

Co-Authored-By: joshua-mir <43032097+joshua-mir@users.noreply.github.com>

* [Hit return to continue]

* ethcore: update hardcoded headers for foundation

* ethcore: update hardcoded headers for ropsten

* ethcore: update hardcoded headers for kovan

* ethcore: use consistant formatting

* ethcore: restore spaces after colon in chain spec

* ethcore: fix bootnodes in chain specs

* ethcore: fix bootnodes in chain specs

* ethcore: enforce newline at the end of chainspecs

* Follow-up to #10105 (#10107)

* HF in POA Sokol (2019-01-04) (#10077)

* HF in POA Sokol (2019-01-04)

poanetwork/poa-chain-spec#91

* Update POA Core bootnodes

* Autogen docs for the "Configuring Parity Ethereum" wiki page. (#10067)

* publish docs changes for autogen config docs

* Update publish-docs.sh

adding an environment variable so js knows not to download git master and just grab the local repo

* Update publish-docs.sh

made some changes making this unnecessary

* fix env variable

env variable passes to node properly now

* use yarn

* test pipeline, revert me

* fix test pipeline, revert me

* change runner tag

* change runner tag 2

* change runner tag

* global git config

* supress upload_files output

* Update .gitlab-ci.yml

reverting testing changes

* Replace tag if exists

Very unlikely to be important/useful

* Autogen docs for the "Configuring Parity Ethereum" wiki page. (#10067)

* publish docs changes for autogen config docs

* Update publish-docs.sh

adding an environment variable so js knows not to download git master and just grab the local repo

* Update publish-docs.sh

made some changes making this unnecessary

* fix env variable

env variable passes to node properly now

* use yarn

* test pipeline, revert me

* fix test pipeline, revert me

* change runner tag

* change runner tag 2

* change runner tag

* global git config

* supress upload_files output

* Update .gitlab-ci.yml

reverting testing changes

* Replace tag if exists

Very unlikely to be important/useful

* finality: dont require chain head to be in the chain (#10054)

* Fill transaction hash on ethGetLog of light client. (#9938)

* Fill transaction hash on ethGetLog of light client. This is enifficient
but we keep align with spec.

* Using better variables names.

* Expose old fast light get log as `parity_getLogsLight`.

* Update rpc/src/v1/helpers/light_fetch.rs

Fix indent.

Co-Authored-By: cheme <emericchevalier.pro@gmail.com>

* Factor common code between light_logs and logs.

* Remove useless check

* Rename parity light logs to 'parity_getLogsNoTransactionHash'.
Fix indent and extra white lines.

* Use vec instead of tree map to avoid inner function.

* better loop

* fix pubsub new_blocks notifications to include all blocks (#9987)

Fix: new blocks notifications sometimes missing in pubsub RPC

Implement new struct to pass to `new_blocks()` with extra parameter - `has_more_blocks_to_import`, which was previously used to determine whether the notification should be sent. Now it's up to each implementation to decide what to do.

Updated all implementations to behave as before, except `eth_pubsub`, which will send notification even when the queue is not empty.

Update tests.

* Revert part of 70a6db7

* HF in POA Core (2019-01-18) - Constantinople (#10155)

poanetwork/poa-chain-spec#100

* ci: re-enable snap publishing (#10142)

* ci: enable snap publishing~

* ci: add publish snap script

* ci: add snapcraft skeleton

* ci: group export statements

* ci: enable snaps on pr branch

* ci: enable snaps on pr branch

* ci: set default BUILD_ARCH

* ci: enable snaps on pr branch

* ci: enable snaps on pr branch

* ci: add libdb to snap

* ci: reinitiate gitlabci

* ci: reinitiate publish-snap script

* ci: fix yaml syntax

* cargo/gitlab env vars

* debug, revert me

* version?

* debug vars

* vars

* vars fix

* vars fix

* revert

* Update scripts/gitlab/publish-snap.sh

Co-Authored-By: 5chdn <5chdn@users.noreply.github.com>

* ci: read track from cargo toml

* Make sure parent block is not in importing queue when importing ancient blocks (#10138)

* Make sure parent block is not in importing queue when importing ancient blocks

* Clear queue when an ancient import fails

* Lock only once in clear

* Add comments why queued check is needed

* Should push the value back to the queue

* Directly check in chain.read()

* Remove extra empty line

* Revert unused verification change
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
F2-bug 🐞 The client fails to follow expected behavior. M4-core ⛓ Core client code / Rust. Q2-easy 💃 Can be fixed by copy and pasting from StackOverflow.
Projects
None yet
Milestone
2.4
Development

Successfully merging a pull request may close this issue.

Identity fix openethereum/parity-ethereum
2 participants
@adetante @jam10o-new