Page MenuHomePhabricator
Differential D530

Cache script execution on a per transaction basis
ClosedPublic
Actions

Authored by deadalnix on Sep 6 2017, 12:21.

Details

Reviewers
freetrader
CCulianu
dagurval
Group Reviewers
Restricted Project
Commits
rSTAGINGf132ac32ab97: Cache script execution on a per transaction basis
rABCf132ac32ab97: Cache script execution on a per transaction basis
Summary

This ensure that we do not run the same script twice with the same set of flags. because we check script when accepting a transaction in the mempool, this shoudl ensure that we do not run the script when connecting a new block.

This is based on core's PR10192 , but the structure is somewhat different (the cache is moved in its own file for instance).

Test Plan
make check
../qa/pull-tester/rpc-tests.py -extended

Run a node on testnet for a day, it looks like it all works well.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix created this revision.Sep 6 2017, 12:21
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 6 2017, 12:21
Harbormaster completed remote builds in B841: Diff 1330.Sep 6 2017, 12:21
deadalnix added parent revisions: D527: Check transaction that enter the mempool against the flags required for the current block rather than a minimal set of mandatory flags., D525: Deduplicate SignatureCacheHasher, D524: Various nits in the signature cache machinery.Sep 6 2017, 12:22
deadalnix added a child revision: D531: Add a wrapper over caching input check..Sep 6 2017, 12:59
sickpig added a subscriber: sickpig.Sep 6 2017, 20:04
sickpig added inline comments.
src/script/scriptcache.cpp
1 ↗(On Diff #1330)

missing licence?

deadalnix updated this revision to Diff 1344.Sep 6 2017, 22:57

Fix licence

Harbormaster completed remote builds in B846: Diff 1344.Sep 6 2017, 22:57
dagurval accepted this revision.Sep 7 2017, 19:53
dagurval added a subscriber: dagurval.
dagurval added inline comments.
src/script/scriptcache.cpp
55 ↗(On Diff #1344)

If IsKeyInScriptCache requires cs_main, shouldn't inserting also require cs_main? (AssertLockHeld(cs_main))

This revision is now accepted and ready to land.Sep 7 2017, 19:53
deadalnix added inline comments.Sep 7 2017, 23:15
src/script/scriptcache.cpp
55 ↗(On Diff #1344)

Yes.

Closed by commit rABCf132ac32ab97: Cache script execution on a per transaction basis (authored by deadalnix, committed by deadalnix). · Explain WhySep 7 2017, 23:17
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
2 lines
7 lines
9 lines
script/
33 lines
60 lines
test/
2 lines
52 lines

Diff 1354

View Options

src/Makefile.am

Loading...
View Options

src/cuckoocache.h

Loading...
View Options

src/init.cpp

Loading...
View Options

src/script/scriptcache.h

Loading...
View Options

src/script/scriptcache.cpp

Loading...
View Options

src/test/test_bitcoin.cpp

Loading...
View Options

src/validation.cpp

Loading...