Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Problem: uncommitted chain-abci storage changes may persisted (fixes #885) #1116

Merged
merged 1 commit into from
Feb 24, 2020

Conversation

tomtau
Copy link
Contributor

@tomtau tomtau commented Feb 21, 2020

Solution: for the TX-related ops, added a temporary map in storage:

  • insertions during delivertx will update the temporary map + insert entry into the current db tx
  • at commit, the db tx is written and flushed
  • lookup extended to indicate whether the caller wants to read the uncommitted change (as in delivertx
    -- e.g. if two transactions in the same block want to spend the same utxo,
    only the first one is valid, so processing needs to read the uncommitted change from the first one before processing the second one)
    or committed change (e.g. in queries)

added two quick unit tests (the first one used to fail with "buffered_write" -- "buffered_write" was removed, as inserts are kind of buffered writes)

…rypto-com#885)

Solution: for the TX-related ops, added a temporary map in storage:
- insertions during delivertx will update the temporary map + insert entry into the current db tx
- at commit, the db tx is written and flushed
- lookup extended to indicate whether the caller wants to read the uncommitted change (as in delivertx
-- e.g. if two transactions in the same block want to spend the same utxo,
only the first one is valid, so processing needs to read the uncommitted change from the first one before processing the second one)
or committed change (e.g. in queries)

added two quick unit tests (the first one used to fail with "buffered_write" -- "buffered_write" was removed, as inserts are kind of buffered writes)
@tomtau
Copy link
Contributor Author

tomtau commented Feb 21, 2020

bors try

bors bot added a commit that referenced this pull request Feb 21, 2020
@bors
Copy link
Contributor

bors bot commented Feb 21, 2020

try

Build succeeded

@codecov
Copy link

codecov bot commented Feb 21, 2020

Codecov Report

Merging #1116 into master will increase coverage by 0.17%.
The diff coverage is 97%.

@@            Coverage Diff             @@
##           master    #1116      +/-   ##
==========================================
+ Coverage   64.39%   64.56%   +0.17%     
==========================================
  Files         148      148              
  Lines       19389    19429      +40     
==========================================
+ Hits        12486    12545      +59     
+ Misses       6903     6884      -19
Impacted Files Coverage Δ
chain-abci/src/app/validate_tx.rs 94.52% <ø> (ø) ⬆️
chain-abci/src/app/mod.rs 87.02% <ø> (-0.07%) ⬇️
chain-abci/src/storage/mod.rs 87.5% <100%> (ø) ⬆️
chain-abci/tests/abci_app.rs 94.59% <100%> (ø) ⬆️
chain-storage/src/tx.rs 95.91% <100%> (+2.81%) ⬆️
chain-abci/src/app/query.rs 58.45% <100%> (+0.29%) ⬆️
chain-storage/src/account/mod.rs 94.11% <83.33%> (-0.56%) ⬇️
chain-storage/src/lib.rs 82.75% <96.87%> (+20.06%) ⬆️
chain-core/src/common/merkle_tree.rs 98.79% <0%> (+0.24%) ⬆️

@tomtau
Copy link
Contributor Author

tomtau commented Feb 24, 2020

bors r+

Copy link
Collaborator

@calvinaco calvinaco left a comment

Choose a reason for hiding this comment

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

LG2M

bors bot added a commit that referenced this pull request Feb 24, 2020
1116: Problem: uncommitted chain-abci storage changes may persisted (fixes #885) r=tomtau a=tomtau

Solution: for the TX-related ops, added a temporary map in storage:
- insertions during delivertx will update the temporary map + insert entry into the current db tx
- at commit, the db tx is written and flushed
- lookup extended to indicate whether the caller wants to read the uncommitted change (as in delivertx
-- e.g. if two transactions in the same block want to spend the same utxo,
only the first one is valid, so processing needs to read the uncommitted change from the first one before processing the second one)
or committed change (e.g. in queries)

added two quick unit tests (the first one used to fail with "buffered_write" -- "buffered_write" was removed, as inserts are kind of buffered writes)


1117: Problem (Fix #1069): blocks with multiple txs may fail light client app hash check r=tomtau a=yihuang

Solution:
- Change BTreeMap to IndexMap

Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
Co-authored-by: yihuang <huang@crypto.com>
@bors
Copy link
Contributor

bors bot commented Feb 24, 2020

Build failed (retrying...)

Copy link
Collaborator

@leejw51crypto leejw51crypto left a comment

Choose a reason for hiding this comment

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

lgtm

@bors
Copy link
Contributor

bors bot commented Feb 24, 2020

@bors bors bot merged commit ecf24be into crypto-com:master Feb 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants