Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bridge duplicates transactions after re-establishing connection to Parity #27

Open
akolotov opened this issue Mar 10, 2018 · 5 comments
Assignees

Comments

@akolotov
Copy link
Contributor

The behaviour to re-establish IPC connection after parity re-run was introduced as part of fix for #22 (9c375cc).

But it caused appearance of minor regression: last set of transactions are being sent twice. Here is the logs from the foreign parity instance before it is killed:

2018-03-10 09:02:53     0/25 peers   87 KiB chain 5 MiB db 0 bytes queue 448 bytes sync  RPC:  0 conn,  1 req/s, 175 µs
2018-03-10 09:03:10  Transaction mined (hash e0ac09e000484637fb5af07649d90103edbe6dc48640caa2e81956ac11d88b36)
|-------- deposit() invocation
2018-03-10 09:03:10  Imported #6501 fa4a…cf4e (1 txs, 0.08 Mgas, 0.99 ms, 0.77 KiB)
2018-03-10 09:03:20  Transaction mined (hash 7914bbbcc10c105823689628b9815c340eb1a923433b8e5c64ebd8664df31eeb)
|-------- approveAndCall() invocation
2018-03-10 09:03:20  Imported #6502 f517…7239 (1 txs, 0.06 Mgas, 1.01 ms, 0.83 KiB)
2018-03-10 09:03:25  Transaction mined (hash 2c6324d205e276170013f8f4450a7379998b4b4fb4b06001d3b296257b84e103)
|-------- submitSignature() invocation
2018-03-10 09:03:25  Imported #6503 50b8…91df (1 txs, 0.26 Mgas, 1.12 ms, 1.03 KiB)
2018-03-10 09:03:28     0/25 peers   98 KiB chain 5 MiB db 0 bytes queue 448 bytes sync  RPC:  0 conn,  2 req/s, 186 µs

The database file contains the following after that:

$ cat ../erc20_db.toml
home_contract_address = "0x2ace2268ed7a96713e6cd18e9b2c2ef0c306c22c"
foreign_contract_address = "0x5e702ea5d81e14ba807fdd0ac923e811a12bfef1"
home_deploy = 6209
foreign_deploy = 6488
checked_deposit_relay = 6218
checked_withdraw_relay = 6503
checked_withdraw_confirm = 6503

As soon as the parity instance is killed and run it produces the following logs:

2018-03-10 09:04:29  Starting Parity/v1.9.2-unstable-0feb0bb-20180201/x86_64-linux-gnu/rustc1.20.0
2018-03-10 09:04:29  Keys path /home/koal/parity/keys/PoA_foreign
2018-03-10 09:04:29  DB path /home/koal/parity/PoA_foreign/chains/PoA_foreign/db/d87bc73279457e60
2018-03-10 09:04:29  Path to dapps /home/koal/parity/PoA_foreign/dapps
2018-03-10 09:04:29  State DB configuration: fast
2018-03-10 09:04:29  Operating mode: active
2018-03-10 09:04:29  Configured for PoA_foreign using AuthorityRound engine
2018-03-10 09:04:30  Public node URL: enode://96b8fa10c0504cb3edb182786fcc8baf6cec3ff5bc4b5f9f82a24c49fe6bd5d09b64a8a2a595ec8d215d0523ec254cf31dda6151cf0a4669edc7bb964fd50af8@192.168.1.15:30343
2018-03-10 09:04:35  Transaction mined (hash 2faba8376df4632da7b4a0ce4cb984b7e51cc90849bf94e8a56a635195253537)
|-------- deposit() invocation
2018-03-10 09:04:35  Transaction mined (hash 00da53a569c639617e23854a2fb3e80745690c69230e07df92e7159f1fe1c958)
|-------- submitSignature() invocation
2018-03-10 09:04:35  Imported #6504 783d…e97e (2 txs, 4.20 Mgas, 1.15 ms, 1.23 KiB)

and the database file contains:

$ cat ../erc20_db.toml
home_contract_address = "0x2ace2268ed7a96713e6cd18e9b2c2ef0c306c22c"
foreign_contract_address = "0x5e702ea5d81e14ba807fdd0ac923e811a12bfef1"
home_deploy = 6209
foreign_deploy = 6488
checked_deposit_relay = 6219
checked_withdraw_relay = 6504
checked_withdraw_confirm = 6504

These transactions are handled properly by the contracts that's why double-spend does not happen. But it increases expenses of bridge authorities by producing more transactions.

@akolotov
Copy link
Contributor Author

A preliminary fix can be found here: yrashk@19241a9

@rstormsf
Copy link
Contributor

rstormsf commented Apr 29, 2018

@akolotov
so in order to do this there are 2 strategies:

  • ask contract if he should do it
  • run estimateGas and if it fails, don't submit a tx

@akolotov
Copy link
Contributor Author

The scenario when a transaction was already sent and in txpool of the node and not included in the block yet. In this case both methods suggested above will return that new transaction with the same parameters could be sent but it is not true.
And moreover such kind of approach could be considered as a workaround rather than fix of root cause.

@yrashk
Copy link
Contributor

yrashk commented May 29, 2018

What's our disposition on this?

@akolotov
Copy link
Contributor Author

I think that it could be closed as soon as #84 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants