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

mempool: adjust the rule of conflicting transaction ranking #3031

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

AnnaShaleva
Copy link
Member

Pay for all the conflicts if you'd like to went in. Close #3028.

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #3031 (68b9ff1) into master (9a15d2d) will decrease coverage by 0.13%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #3031      +/-   ##
==========================================
- Coverage   84.70%   84.58%   -0.13%     
==========================================
  Files         328      328              
  Lines       43399    43716     +317     
==========================================
+ Hits        36762    36975     +213     
- Misses       5149     5248      +99     
- Partials     1488     1493       +5     
Impacted Files Coverage Δ
pkg/rpcclient/nep11/base.go 61.19% <0.00%> (-38.81%) ⬇️
pkg/rpcclient/nep17/nep17.go 44.18% <0.00%> (-55.82%) ⬇️
pkg/smartcontract/binding/generate.go 95.23% <88.46%> (-1.46%) ⬇️
pkg/compiler/compiler.go 78.48% <95.52%> (+3.30%) ⬆️
cli/smartcontract/smart_contract.go 91.98% <100.00%> (+0.10%) ⬆️
pkg/compiler/codegen.go 92.01% <100.00%> (ø)
pkg/compiler/debug.go 92.97% <100.00%> (-0.51%) ⬇️
pkg/compiler/inline.go 97.47% <100.00%> (+0.18%) ⬆️
pkg/core/mempool/mem_pool.go 97.12% <100.00%> (+0.03%) ⬆️
pkg/neotest/compile.go 100.00% <100.00%> (ø)
... and 1 more

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

AnnaShaleva added a commit to AnnaShaleva/neo that referenced this pull request Jun 1, 2023
Transaction that conflicts with mempooled transactions have to pay
larger network fee than the sum of all conflicting transactions in
the pool that have the same sender as the newcomer.

Port the nspcc-dev/neo-go#3031.
pkg/core/mempool/mem_pool_test.go Outdated Show resolved Hide resolved
pkg/core/mempool/mem_pool_test.go Show resolved Hide resolved
Pay for all the conflicts if you'd like to went in. Close #3028.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@roman-khimov roman-khimov merged commit 50fad84 into master Jun 1, 2023
12 of 18 checks passed
@roman-khimov roman-khimov deleted the conflicts-priority branch June 1, 2023 10:48
AnnaShaleva added a commit to AnnaShaleva/neo that referenced this pull request Jun 1, 2023
Transaction that conflicts with mempooled transactions have to pay
larger network fee than the sum of all conflicting transactions in
the pool that have the same sender as the newcomer.

Port the nspcc-dev/neo-go#3031.
shargon added a commit to neo-project/neo that referenced this pull request Sep 4, 2023
* Payloads: implement Conflicts attribute

Closes #1991.

* Update src/Neo/Ledger/MemoryPool.cs

* Fix conflicting tx fees accounting

Fix the comment #2818 (comment).

* Reformat code and improve variables naming

Fix Vitor's comments.

* Make comments more clear and adjust variables naming

Fix Owen's feedback.

* Fix Conflicts attribute verification

Consider the case when transaction B has the hash of transaction A
in the Conflicts attribute and transaction B is on-chain. Let the
transaction C has the hash of transaction A in the Conflicts attribute.
Then transaction C still should be able to pass verification and should
be accepted to the subsequent block, because transaction A isn't on chain
and transaction A will never be accepted.

Thanks to Owen for testing, this case is described at the
#2818 (review).
The expected behaviour in the described case is that TXID-D and TXID-E will
be successfully accepted to the mempool and to chain, because the conflicting
TXID-A is not on chain (and it's OK that the hash of TXID-A is on-chain as
the conflicting hash).

* Fix formatting, comments and add a testcase for conflicting mempool txs

* Add one more testcase for conflicting transactions

Testcase provided by Vitor in #2818 (comment).

* Implement economic adjustments for Conflicts attribute

Transaction that conflicts with mempooled transactions have to pay
larger network fee than the sum of all conflicting transactions in
the pool that have the same sender as the newcomer.

Port the nspcc-dev/neo-go#3031.

* Remove Trimmed value

* Check signers of on-chained conflict during new tx verification

Fix the problem described in
#2818 (review).

During new transaction verification if there's an on-chain conflicting
transaction, we should check the signers of this conflicting transaction.
If the signers contain payer of the incoming transaction, then the conflict
is treated as valid and verification for new incoming transaction should
fail. Otherwise, the conflict is treated as the malicious attack attempt
and will not be taken into account; verification for the new incoming
transaction should continue in this case.

* Properly handle duplicating Conflicts hashes from the persisted transactions

* Store signers of all conflicting on-chain transactions with the same Conflicts attribute

Store not only signers of the latest conflicting on-chain transaction, but
signers of all conflicting transactions with the same attribute.

* MemoryPool: consider signers intersection on Conflicts check

This patch should be a part of fd1748d, but
was accidentally skipped. This patch matches exactly the NeoGo behaviour that
was added in nspcc-dev/neo-go#3061 and that was
discussed earlier in #2818 (comment).

Fix the issue described in #2818 (comment).

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>

* MemoryPool: add test for on-chain conflict

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>

* Clean using

* Refactor Malicious_OnChain_Conflict test

Move it to the UT_Blockchain.cs file and refactor it a bit to make it
actually pass as expected.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>

* Use Distinct to filter out duplicating conflicting on-chain signers

Co-authored-by: Shargon <shargon@gmail.com>

* Use HashSet as a container of conflicting hashes in the mempool

Fix #2818 (comment).

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>

---------

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Co-authored-by: Shargon <shargon@gmail.com>
Co-authored-by: Vitor Nazário Coelho <vncoelho@gmail.com>
Co-authored-by: Jimmy <jinghui@wayne.edu>
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.

Implement economic adjustments for Conflicts
2 participants