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

feat: ABCI 1.0 baseapp integration #13453

Merged
merged 261 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from 250 commits
Commits
Show all changes
261 commits
Select commit Hold shift + click to select a range
d72d1a2
size impl
kocubinski Sep 13, 2022
01ed963
cleanup
kocubinski Sep 13, 2022
35ff6d6
fix nill pointer when checking if key exists
JeancarloBarrios Sep 14, 2022
bbd9405
add init of hashes
JeancarloBarrios Sep 14, 2022
3cc3fb1
notes
kocubinski Sep 14, 2022
9ffa994
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Sep 14, 2022
aa103ef
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Sep 14, 2022
77a9b37
btree ordering
kocubinski Sep 14, 2022
46c34ff
MauricGit/skiplist impl
kocubinski Sep 14, 2022
9a4eeac
add huandu skiplist impl
kocubinski Sep 14, 2022
09f4e6b
very rough draft of stateful ordering
kocubinski Sep 15, 2022
a6fb1e5
some notes
kocubinski Sep 16, 2022
c8931c9
fill out remove method
kocubinski Sep 16, 2022
c3746c7
some comments
kocubinski Sep 16, 2022
816dc70
mempool to own package
kocubinski Sep 18, 2022
85c5f1e
stateful mempool test passes
kocubinski Sep 18, 2022
a10b047
dfs impl in graph test passing
kocubinski Sep 19, 2022
59b2195
comment
kocubinski Sep 19, 2022
aafb4ce
add test case
kocubinski Sep 19, 2022
1798d68
runtime edge detection is a bad idea
kocubinski Sep 22, 2022
6be3812
feat(mempool): tests and benchmark for mempool (#13273)
JeancarloBarrios Sep 22, 2022
5f2b9cc
Refactor mempool impl
kocubinski Sep 22, 2022
14afa8d
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Sep 22, 2022
ee97a25
tx generation
kocubinski Sep 23, 2022
bfda138
shuffle return
kocubinski Sep 23, 2022
ee2ed7e
transaction generating
kocubinski Sep 24, 2022
afc04eb
clean up tx order tests
kocubinski Sep 25, 2022
a9a8fa3
generative tests are passing
kocubinski Sep 25, 2022
9556eff
add comment
kocubinski Sep 25, 2022
87bf3b6
more iterations
kocubinski Sep 25, 2022
5b647eb
clean up
kocubinski Sep 25, 2022
6407c12
even more tests of tests
kocubinski Sep 25, 2022
629d17b
notes on multi sender handling
kocubinski Sep 25, 2022
0bd11ff
rework tests
kocubinski Sep 28, 2022
01858d2
hypothesis for multi sender
kocubinski Sep 28, 2022
ed23ead
refactoring into multi sender testing
kocubinski Sep 28, 2022
fc784dc
this approach for multi sender is not excellent
kocubinski Sep 28, 2022
1758aae
one more test case
kocubinski Sep 28, 2022
0ec9335
ate a bag of peanut butter m&ms, re-writing the graph.go impl
kocubinski Sep 29, 2022
1520b5d
rewrite graph
kocubinski Sep 29, 2022
fa7cc9a
add an interesting case which breaks current mempool impl
kocubinski Sep 29, 2022
c4495fd
all the edges
kocubinski Sep 30, 2022
f4ff6c6
work
kocubinski Sep 30, 2022
f57847d
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 2, 2022
82f701c
go mod tidy
kocubinski Oct 2, 2022
16100f9
fix graph initial conditions finding
kocubinski Oct 3, 2022
582e914
refactoring tests
kocubinski Oct 3, 2022
a8d58dd
derive sender from pubkey in signer_infos
kocubinski Oct 3, 2022
1e1be0e
refactoring
kocubinski Oct 4, 2022
b749c6f
baseapp integration
kocubinski Oct 4, 2022
26eb530
test
JeancarloBarrios Oct 4, 2022
5143d58
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
JeancarloBarrios Oct 4, 2022
90996c7
Procees proposal base
JeancarloBarrios Oct 4, 2022
5be329f
more baseapp integration
kocubinski Oct 5, 2022
c25385a
cd simapp && go mod tidy
kocubinski Oct 5, 2022
1725e8d
fixing tests
kocubinski Oct 5, 2022
7cf3a52
remove graph impl
kocubinski Oct 5, 2022
9c44433
Default mempool implementation
kocubinski Oct 5, 2022
15834d7
Update go.mod
kocubinski Oct 5, 2022
7457271
Remove moved file
kocubinski Oct 5, 2022
b0b8b27
backporting from integration branch
kocubinski Oct 5, 2022
5eac4b7
cd tests && go mod tidy
kocubinski Oct 5, 2022
834f85f
test refactor
kocubinski Oct 5, 2022
d90ac72
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 5, 2022
fbe1bb4
resolve conflicts
kocubinski Oct 5, 2022
ffe535d
mempool tests passing
kocubinski Oct 5, 2022
056bca1
remove type conversion
kocubinski Oct 5, 2022
78b774c
test cleanup
kocubinski Oct 5, 2022
192d421
rename parallel impl
kocubinski Oct 5, 2022
7adc7bf
more cleanup
kocubinski Oct 5, 2022
bd1b55f
go imports
kocubinski Oct 5, 2022
96cd0c7
Add some godoc strings
kocubinski Oct 5, 2022
83dcbc7
typo
kocubinski Oct 5, 2022
e473bcf
more godoc
kocubinski Oct 5, 2022
613019a
comment format
kocubinski Oct 5, 2022
3099039
typo
kocubinski Oct 5, 2022
295703f
update godoc
kocubinski Oct 6, 2022
47fad32
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 6, 2022
302f81e
simple benchmark
JeancarloBarrios Oct 6, 2022
83d6320
test cleanup
kocubinski Oct 6, 2022
873cb74
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 6, 2022
5849815
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 10, 2022
e61ceea
move default implementation to default.go
kocubinski Oct 10, 2022
48ce41c
minor clean up to default impl
kocubinski Oct 10, 2022
c24a152
minor refactoring, add skip list test
kocubinski Oct 10, 2022
ad829d4
refactor away iterations tracking
kocubinski Oct 10, 2022
8d13cc2
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 10, 2022
869b524
begin baseapp integration
kocubinski Oct 10, 2022
47ba2c2
change not found error
kocubinski Oct 10, 2022
541f112
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 10, 2022
bae75b9
Merge branch 'kocubinski/mempool-impl' into kocubinski/mempool-baseapp
kocubinski Oct 10, 2022
af4f5bc
abci1.0 methods
kocubinski Oct 10, 2022
1d5b1d4
fix import format
kocubinski Oct 10, 2022
de40135
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 11, 2022
6c15d92
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 11, 2022
250ea90
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Oct 11, 2022
5be06eb
more integration
kocubinski Oct 11, 2022
2663dc2
Merge branch 'kocubinski/mempool-baseapp' of github.com:cosmos/cosmos…
kocubinski Oct 11, 2022
c2e7069
rename default impl -> priority
kocubinski Oct 11, 2022
42d52c1
add default constructor
kocubinski Oct 11, 2022
4ecf2e2
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Oct 11, 2022
58bd2ce
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Oct 11, 2022
00a7a8f
default supplier in simapp
kocubinski Oct 11, 2022
be180e8
need a signature for removal in tx tests
kocubinski Oct 11, 2022
9d120c1
all tx in deliver_tx_test now have a sig
kocubinski Oct 11, 2022
4f466a4
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 11, 2022
abbe016
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 11, 2022
d2f8b21
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 12, 2022
63d6606
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 12, 2022
91e30d7
remove unused test file
kocubinski Oct 12, 2022
ea8589d
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Oct 12, 2022
d02e4a9
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Oct 12, 2022
7d2fd41
Update types/mempool/mempool_test.go
JeancarloBarrios Oct 12, 2022
1683445
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
JeancarloBarrios Oct 12, 2022
b9a2e05
smiple benchmarks
JeancarloBarrios Oct 12, 2022
a1f2ae0
Use checkState
facundomedica Oct 16, 2022
5bd05f8
simple fix
JeancarloBarrios Oct 17, 2022
0c37d23
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 17, 2022
19e6e3e
update algorithm for failing test case
kocubinski Oct 18, 2022
5cc97f4
golint fix
kocubinski Oct 18, 2022
2bc00a1
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 18, 2022
e42d6a8
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 18, 2022
1366a87
fix gosec warning
kocubinski Oct 18, 2022
f2457fa
working solution
kocubinski Oct 18, 2022
d7dc83a
remove debug prints
kocubinski Oct 18, 2022
bfee335
add priority ties test
kocubinski Oct 18, 2022
b7fe34c
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 18, 2022
52e10e6
comments
kocubinski Oct 18, 2022
4e429dd
Add godoc to mempool.Size implementation
kocubinski Oct 18, 2022
5c80aaf
increment
kocubinski Oct 18, 2022
46fb8c4
add another test case
kocubinski Oct 18, 2022
a17f506
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 18, 2022
078168b
fix gosec error
kocubinski Oct 18, 2022
59cc71f
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 18, 2022
e321ec2
implement missing methods for tx to pass app test
JeancarloBarrios Oct 19, 2022
db1f4cd
Merge branch 'main' into kocubinski/mempool-all
JeancarloBarrios Oct 19, 2022
ba57a58
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 19, 2022
227ea17
Update types/mempool/priority.go
alexanderbez Oct 19, 2022
522aaab
bez: lint/godoc
alexanderbez Oct 19, 2022
7021bdd
bez: lint/godoc
alexanderbez Oct 19, 2022
81c67f7
bez: lint/godoc
alexanderbez Oct 19, 2022
af919d3
Update types/mempool/priority.go
kocubinski Oct 19, 2022
54837e8
remove line
kocubinski Oct 19, 2022
e9a7232
set txSize to 0 in tx/builder.go setters
kocubinski Oct 19, 2022
f634306
some comments
kocubinski Oct 20, 2022
6034696
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 20, 2022
fb6eb64
log out seeds in tests
kocubinski Oct 20, 2022
838e126
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
kocubinski Oct 20, 2022
c043a7a
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 20, 2022
b07a56c
Merge branch 'main' into kocubinski/mempool-all
JeancarloBarrios Oct 20, 2022
45044bd
add a comment
kocubinski Oct 21, 2022
5ccf004
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 21, 2022
0528e05
Fix inconsistency in setting baseapp mempool
kocubinski Oct 21, 2022
1cb53a0
Merge branch 'main' into kocubinski/mempool-impl
kocubinski Oct 23, 2022
cc608b6
added function for abci integration
JeancarloBarrios Oct 24, 2022
2f56b6d
abci base methods for abci++
JeancarloBarrios Oct 24, 2022
bab393a
priority mempool
JeancarloBarrios Oct 24, 2022
b88614e
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
JeancarloBarrios Oct 24, 2022
2afe7b1
Merge branch 'kocubinski/mempool-impl' of github.com:cosmos/cosmos-sd…
JeancarloBarrios Oct 24, 2022
1dfe713
Merge branch 'kocubinski/mempool-impl' into kocubinski/mempool-all
JeancarloBarrios Oct 24, 2022
5fbee38
simple for testing purposes
JeancarloBarrios Oct 24, 2022
94ddfa2
prepare and process propossal have the same logic if a tx fails they …
JeancarloBarrios Oct 24, 2022
f6c073f
small fix
JeancarloBarrios Oct 24, 2022
179869e
Merge branch 'main' into kocubinski/mempool-all
JeancarloBarrios Oct 26, 2022
b1c84cb
base abci changes
JeancarloBarrios Oct 26, 2022
e514f7b
t
JeancarloBarrios Oct 27, 2022
c834047
t
JeancarloBarrios Oct 27, 2022
4d11de2
set prepare and process like check state
JeancarloBarrios Oct 27, 2022
4d295b1
deletead simple due to it being merge to main in a separate PR
JeancarloBarrios Oct 27, 2022
46ebfc4
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocubinski/m…
kocubinski Oct 27, 2022
ffdb934
draft of select cursor
kocubinski Oct 27, 2022
55d13d7
fix reference to deleted file
kocubinski Oct 27, 2022
3f76cb6
panic
kocubinski Oct 27, 2022
cb147a3
handle empty case properly
kocubinski Oct 27, 2022
b04e051
feat: mempool select iterator (#13677)
kocubinski Oct 27, 2022
6f71213
t
JeancarloBarrios Oct 27, 2022
0cc1aaa
refactor iterator
kocubinski Oct 27, 2022
732d1b1
Merge branch 'kocubinski/mempool-select-cursor' into kocubinski/mempo…
kocubinski Oct 27, 2022
6601cf3
t
JeancarloBarrios Oct 28, 2022
96cb90b
Remove mempool.Tx interface
kocubinski Oct 31, 2022
657ce24
error handling
kocubinski Oct 31, 2022
736306c
Add sdk.Context to Mempool.Select()
kocubinski Nov 1, 2022
e907d77
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 1, 2022
61bd98a
fix whitespace
kocubinski Nov 1, 2022
1f53170
revert simapp/app.go
kocubinski Nov 1, 2022
369c622
fix whitespace
kocubinski Nov 1, 2022
7a7504a
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 1, 2022
32899a5
use setter for consistency
kocubinski Nov 1, 2022
ddec83e
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
kocubinski Nov 1, 2022
3e6e1a4
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 2, 2022
9c23184
test advances
JeancarloBarrios Nov 2, 2022
3f1dcd8
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
JeancarloBarrios Nov 2, 2022
d4234d8
test construction
kocubinski Nov 2, 2022
580c8b3
remove prints
JeancarloBarrios Nov 2, 2022
3c8640e
fixed comment
JeancarloBarrios Nov 2, 2022
d1a1cf8
add tests exercising multiple signers with real messages
kocubinski Nov 2, 2022
0697e88
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 2, 2022
fc4bccb
happy path test
kocubinski Nov 2, 2022
659faa3
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
kocubinski Nov 2, 2022
10cd747
move to test suite
kocubinski Nov 2, 2022
bfe7faf
add failing antehandler test
kocubinski Nov 2, 2022
de876ca
test max byte sizae
JeancarloBarrios Nov 2, 2022
51e0da9
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 3, 2022
7dc0684
Update baseapp/abci.go
JeancarloBarrios Nov 3, 2022
72a5c3c
Update server/types/app.go
JeancarloBarrios Nov 3, 2022
201d52e
Update baseapp/baseapp.go
JeancarloBarrios Nov 3, 2022
dea715f
Update baseapp/baseapp.go
JeancarloBarrios Nov 3, 2022
0af1899
remove interface implemantation that is no longer needed
JeancarloBarrios Nov 3, 2022
3134207
fix
JeancarloBarrios Nov 3, 2022
9e3048f
Allow applications to define process proposal
kocubinski Nov 3, 2022
41816be
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
kocubinski Nov 3, 2022
678b555
wrapping method
kocubinski Nov 3, 2022
a75e4ef
t
JeancarloBarrios Nov 3, 2022
215d2a6
t
JeancarloBarrios Nov 3, 2022
587f0fb
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
JeancarloBarrios Nov 3, 2022
88f574d
use a no-op message server
kocubinski Nov 3, 2022
7180e94
clean up imports
kocubinski Nov 3, 2022
97a9c76
removed unesesary state set
JeancarloBarrios Nov 3, 2022
6e5ff1c
t
JeancarloBarrios Nov 3, 2022
e1d5ed4
t
JeancarloBarrios Nov 3, 2022
8d4e97b
t
JeancarloBarrios Nov 3, 2022
99566ce
Use correct state in ProcessProposal
kocubinski Nov 4, 2022
16e98c9
Revert "removed unesesary state set"
kocubinski Nov 4, 2022
a15d9f0
Set Block height
kocubinski Nov 4, 2022
cf88b04
Fix processProposal handler
kocubinski Nov 4, 2022
5db4724
Remove context modification in BeginBlock
kocubinski Nov 4, 2022
0696b82
minor cleanup
kocubinski Nov 4, 2022
a8dd44d
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 4, 2022
ef5a625
t
JeancarloBarrios Nov 4, 2022
3ac1d30
fix go imports
kocubinski Nov 7, 2022
deb33f5
Merge branch 'kocubinski/mempool-all' of github.com:cosmos/cosmos-sdk…
kocubinski Nov 7, 2022
74775b6
Update baseapp/baseapp.go
alexanderbez Nov 7, 2022
1e36da8
Apply suggestions from code review
alexanderbez Nov 7, 2022
a3bc195
Update baseapp/baseapp.go
JeancarloBarrios Nov 7, 2022
ccc00f1
duplicate ProcessProposal request fields into context object
kocubinski Nov 7, 2022
5595988
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 7, 2022
26641b6
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 7, 2022
061e68e
debug and fix bug surfaced in liveness tests
kocubinski Nov 8, 2022
e646e46
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 8, 2022
e594b59
init prepare/process proposal state in .Init
kocubinski Nov 8, 2022
40a5b21
reject when checktx fails on process propossal
JeancarloBarrios Nov 8, 2022
e50ef7d
reject when checktx fails on process propossal
JeancarloBarrios Nov 8, 2022
5ce1d3a
Add 2 comments
kocubinski Nov 8, 2022
97f02e9
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 8, 2022
c10e857
Update baseapp/abci.go
kocubinski Nov 8, 2022
323b9f7
fix comment
kocubinski Nov 8, 2022
3575579
Update baseapp/abci.go
alexanderbez Nov 8, 2022
348f67e
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 8, 2022
4576305
Update baseapp/baseapp.go
kocubinski Nov 8, 2022
61e1b60
Merge branch 'main' into kocubinski/mempool-all
kocubinski Nov 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 58 additions & 11 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/mempool"
)

// Supported ABCI Query prefixes
Expand All @@ -38,6 +39,8 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
// req.InitialHeight is 1 by default.
initHeader := tmproto.Header{ChainID: req.ChainId, Time: req.Time}

app.logger.Info("InitChain", "initialHeight", req.InitialHeight, "chainID", req.ChainId)

// If req.InitialHeight is > 1, then we set the initial version in the
// stores.
if req.InitialHeight > 1 {
Expand All @@ -49,9 +52,11 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
}
}

// initialize the deliver state and check state with a correct header
// initialize states with a correct header
app.setDeliverState(initHeader)
app.setCheckState(initHeader)
app.setPrepareProposalState(initHeader)
app.setProcessProposalState(initHeader)

// Store the consensus params in the BaseApp's paramstore. Note, this must be
// done after the deliver state and context have been set as it's persisted
Expand Down Expand Up @@ -182,8 +187,6 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
WithHeaderHash(req.Hash).
WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx))

// we also set block gas meter to checkState in case the application needs to
// verify gas consumption during (Re)CheckTx
if app.checkState != nil {
app.checkState.ctx = app.checkState.ctx.
WithBlockGasMeter(gasMeter).
Expand Down Expand Up @@ -238,19 +241,50 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc
// work in a block before proposing it.
//
// Transactions can be modified, removed, or added by the application. Since the
// application maintains it's own local mempool, it will ignore the transactions
// application maintains its own local mempool, it will ignore the transactions
// provided to it in RequestPrepareProposal. Instead, it will determine which
// transactions to return based on the mempool's semantics and the MaxTxBytes
// provided by the client's request.
//
// Note, there is no need to execute the transactions for validity as they have
// already passed CheckTx.
//
// Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md
// Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md
func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePrepareProposal {
// TODO: Implement.
return abci.ResponsePrepareProposal{Txs: req.Txs}
var (
txsBytes [][]byte
byteCount int64
)

ctx := app.getContextForTx(runTxPrepareProposal, []byte{})
iterator := app.mempool.Select(ctx, req.Txs)

for iterator != nil {
memTx := iterator.Tx()

bz, err := app.txEncoder(memTx)
if err != nil {
panic(err)
}

txSize := int64(len(bz))

_, _, _, _, err = app.runTx(runTxPrepareProposal, bz)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't necessarily understand the point of forcing the runTx behavior on all app-side mempool implementations. If anything, this just makes the SetMempool API more restrictive. It also forces the performance burden on all custom app-side mempools of running all transactions through the Antehandlers a third time (CheckTx, DeliverTx, and now PrepareProposal). As stated in the comment here this shouldn't be necessary unless the application developer's custom app-side mempool implementation has a bug or is trying to do something unexpected. They can already break themselves in other ways, for example a developer could add some panic to their Select implementation which means no proposal would ever be sent, right?

Just a suggestion and not a dealbreaker, but I think I'd prefer to be able to opt out of this somehow.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • AnteHandlers add no performance burden -- that's the entire point of them. Lightweight checks prior to tx execution.
  • We add validity checks here stated per the ADR. In addition, it's possible for an app to include/inject txs on Insert during CheckTx so this ensures any injected txs are still valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree that they add no performance burden. Some of the checks read from state which could potentially mean disk reads if the state is not in cache. There is also signature verification which is not free.

A couple of examples of reading from state during Antehandlers:

  • Reading the Account in ConsumeTxSizeGasDecorator here.
  • Calling SendCoinsFromAccountToModule in DeductFees which reads module accounts from state here.

There is also signature verifications here which looks to take ~0.15ms per-op for secp256k1 if I am understanding the benchmarks correctly (Sig/secp256k1-8). This means while ABCI is globally locked during PrepareProposal, you're doing this operation for each transaction serially. So if the block has 100 transactions on average, you're spending ~15ms on average per-block blocking the entire application and verifying transaction signatures that have already been verified.

Let me know if I'm misunderstanding something!

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree that they add no performance burden. Some of the checks read from state which could potentially mean disk reads if the state is not in cache. There is also signature verification which is not free.

True, state is read from but not written to. In the grand scheme of things, the performance overhead is near negligible.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, state is read from but not written to. In the grand scheme of things, the performance overhead is near negligible.

Just to be super clear, 15 milliseconds of additional global lock time per-block (as in my above signature example) is definitely not negligible for us, especially given this is totally unnecessary computation in our case. It's very likely we're a unique case given our throughput aspirations so therefore your assessment is probably true for most other chains.

Copy link
Member

@tac0turtle tac0turtle Nov 9, 2022

Choose a reason for hiding this comment

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

agree with @prettymuchbryce, on profiles I have run on live networks ante handlers show up quite often and do add a non negligible amount of overhead. For many chains this isn't a problem but it's not something we should over look as we are trying to make the sdk more performant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Again, we validate the txs (i.e. AnteHandler) since applications can insert txs on Insert during CheckTx and we need to ensure these are still valid when selecting during PrepareProposal.

Also, recall:

  1. The application defines the AnteHandler chain
  2. The application and thus it's AnteHandler is aware of tx execution context (i.e. PrepareProposal)

So if this doesn't suite your needs, your AnteHandler chain can easily do something like "If PrepareProposal, skip XYZ".

Copy link
Contributor

Choose a reason for hiding this comment

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

The application defines the AnteHandler chain
So if this doesn't suite your needs, your AnteHandler chain can easily do something like "If PrepareProposal, skip XYZ".

This is a really good point that I had forgotten. I think this would be reasonable solution.

if err != nil {
err := app.mempool.Remove(memTx)
if err != nil && !errors.Is(err, mempool.ErrTxNotFound) {
panic(err)
}
iterator = iterator.Next()
continue
} else if byteCount += txSize; byteCount <= req.MaxTxBytes {
txsBytes = append(txsBytes, bz)
} else {
break
Copy link
Contributor

Choose a reason for hiding this comment

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

Food for thought:

This else block seems to suggest that the txs queued up in the iterator won't be part of the returned response once the MaxTxBytes has been reached. Not processing all the queued up txs is not desirable in certain app scenarios.

For example, let's say Select returns txs [A1, A2, B1, B2] but PrepareProposal only processes [A1, A2] and leaves out [B1, B2], because the MaxTxBytes has been reached. Select returned the txs in the above order, because the app has an invariant that txs of type A must appear before txs of type B, but from the app's perspective, txs of type B are much more important to include in the block than type A. So in this case, the invariant is respected, but the app doesn't get to include txs of type B and is not alerted about this behavior.

Thinking out loud, to prevent this issue, the app can validate that the txs returned in Select is under the MaxTxBytes cap. However, the app doesn't get to learn if all the txs returned in Select were processed or not (it speculates that the txs were processed by PrepareProposal, but there's no explicit signal).

Thinking out loud, some suggestions:

  • Hint MaxTxBytes as part of Select
  • Add a way for the app to learn that not all txs returned in Select were processed by PrepareProposal and retry Select / PrepareProposal
  • Add a mode where "process all txs returned by Select or fail"

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason we don't pass MaxBytes to Select is that we need to ensure that selected txs are valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed this further offline. #13831 fixes the concern.

}

iterator = iterator.Next()
}

return abci.ResponsePrepareProposal{Txs: txsBytes}
}

// ProcessProposal implements the ProcessProposal ABCI method and returns a
Expand All @@ -266,8 +300,19 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) abci.Respon
// Ref: https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-060-abci-1.0.md
// Ref: https://github.com/tendermint/tendermint/blob/main/spec/abci/abci%2B%2B_basic_concepts.md
func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) abci.ResponseProcessProposal {
// TODO: Implement.
return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}
if app.processProposal == nil {
panic("app.ProcessProposal is not set")
}

ctx := app.processProposalState.ctx.
WithVoteInfos(app.voteInfos).
WithBlockHeight(req.Height).
WithBlockTime(req.Time).
WithHeaderHash(req.Hash).
WithProposer(req.ProposerAddress)
ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx))
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
kocubinski marked this conversation as resolved.
Show resolved Hide resolved

return app.processProposal(ctx, req)
}

// CheckTx implements the ABCI interface and executes a tx in CheckTx mode. In
Expand Down Expand Up @@ -367,6 +412,8 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) {
// NOTE: This is safe because Tendermint holds a lock on the mempool for
// Commit. Use the header from this latest block.
app.setCheckState(header)
app.setPrepareProposalState(header)
app.setProcessProposalState(header)

// empty/reset the deliver state
app.deliverState = nil
Expand Down
196 changes: 196 additions & 0 deletions baseapp/abci_v1_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package baseapp_test

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
abci "github.com/tendermint/tendermint/abci/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"cosmossdk.io/depinject"
"github.com/cosmos/cosmos-sdk/baseapp"
baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/mempool"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
)

type NoopCounterServerImpl struct{}

func (m NoopCounterServerImpl) IncrementCounter(
_ context.Context,
_ *baseapptestutil.MsgCounter,
) (*baseapptestutil.MsgCreateCounterResponse, error) {
return &baseapptestutil.MsgCreateCounterResponse{}, nil
}

type ABCIv1TestSuite struct {
suite.Suite
baseApp *baseapp.BaseApp
mempool mempool.Mempool
txConfig client.TxConfig
cdc codec.ProtoCodecMarshaler
}

func TestABCIv1TestSuite(t *testing.T) {
suite.Run(t, new(ABCIv1TestSuite))
}

func (s *ABCIv1TestSuite) SetupTest() {
t := s.T()
anteKey := []byte("ante-key")
pool := mempool.NewNonceMempool()
anteOpt := func(bapp *baseapp.BaseApp) {
bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey))
}

var (
appBuilder *runtime.AppBuilder
cdc codec.ProtoCodecMarshaler
registry codectypes.InterfaceRegistry
)
err := depinject.Inject(makeMinimalConfig(), &appBuilder, &cdc, &registry)
require.NoError(t, err)

app := setupBaseApp(t, anteOpt, baseapp.SetMempool(pool))
baseapptestutil.RegisterInterfaces(registry)
app.SetMsgServiceRouter(baseapp.NewMsgServiceRouter())
app.SetInterfaceRegistry(registry)

baseapptestutil.RegisterKeyValueServer(app.MsgServiceRouter(), MsgKeyValueImpl{})
baseapptestutil.RegisterCounterServer(app.MsgServiceRouter(), NoopCounterServerImpl{})
header := tmproto.Header{Height: app.LastBlockHeight() + 1}

app.InitChain(abci.RequestInitChain{
ConsensusParams: &tmproto.ConsensusParams{},
})

app.BeginBlock(abci.RequestBeginBlock{Header: header})

// patch in TxConfig insted of using an output from x/auth/tx
txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes)

app.SetTxDecoder(txConfig.TxDecoder())
app.SetTxEncoder(txConfig.TxEncoder())

s.baseApp = app
s.mempool = pool
s.txConfig = txConfig
s.cdc = cdc
}

func (s *ABCIv1TestSuite) TestABCIv1_HappyPath() {
txConfig := s.txConfig
t := s.T()

tx := newTxCounter(txConfig, 0, 1)
txBytes, err := txConfig.TxEncoder()(tx)
require.NoError(t, err)

reqCheckTx := abci.RequestCheckTx{
Tx: txBytes,
Type: abci.CheckTxType_New,
}
s.baseApp.CheckTx(reqCheckTx)

tx2 := newTxCounter(txConfig, 1, 1)

tx2Bytes, err := txConfig.TxEncoder()(tx2)
require.NoError(t, err)

err = s.mempool.Insert(sdk.Context{}, tx2)
require.NoError(t, err)
reqPreparePropossal := abci.RequestPrepareProposal{
MaxTxBytes: 1000,
}
resPreparePropossal := s.baseApp.PrepareProposal(reqPreparePropossal)

require.Equal(t, 2, len(resPreparePropossal.Txs))

var reqProposalTxBytes [2][]byte
reqProposalTxBytes[0] = txBytes
reqProposalTxBytes[1] = tx2Bytes
reqProcessProposal := abci.RequestProcessProposal{
Txs: reqProposalTxBytes[:],
}

s.baseApp.SetProcessProposal(nil)
require.Panics(t, func() { s.baseApp.ProcessProposal(reqProcessProposal) })
s.baseApp.SetProcessProposal(s.baseApp.DefaultProcessProposal())

resProcessProposal := s.baseApp.ProcessProposal(reqProcessProposal)
require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status)

res := s.baseApp.DeliverTx(abci.RequestDeliverTx{Tx: txBytes})
require.Equal(t, 1, s.mempool.CountTx())

require.NotEmpty(t, res.Events)
require.True(t, res.IsOK(), fmt.Sprintf("%v", res))
}

func (s *ABCIv1TestSuite) TestABCIv1_PrepareProposal_ReachedMaxBytes() {
txConfig := s.txConfig
t := s.T()

for i := 0; i < 100; i++ {
tx2 := newTxCounter(txConfig, int64(i), int64(i))
err := s.mempool.Insert(sdk.Context{}, tx2)
require.NoError(t, err)
}

reqPreparePropossal := abci.RequestPrepareProposal{
MaxTxBytes: 1500,
}
resPreparePropossal := s.baseApp.PrepareProposal(reqPreparePropossal)

require.Equal(t, 10, len(resPreparePropossal.Txs))
}

func (s *ABCIv1TestSuite) TestABCIv1_PrepareProposal_BadEncoding() {
txConfig := authtx.NewTxConfig(s.cdc, authtx.DefaultSignModes)

t := s.T()

tx := newTxCounter(txConfig, 0, 0)
err := s.mempool.Insert(sdk.Context{}, tx)
require.NoError(t, err)

reqPrepareProposal := abci.RequestPrepareProposal{
MaxTxBytes: 1000,
}
resPrepareProposal := s.baseApp.PrepareProposal(reqPrepareProposal)

require.Equal(t, 1, len(resPrepareProposal.Txs))
}

func (s *ABCIv1TestSuite) TestABCIv1_PrepareProposal_Failures() {
tx := newTxCounter(s.txConfig, 0, 0)
txBytes, err := s.txConfig.TxEncoder()(tx)
s.NoError(err)

reqCheckTx := abci.RequestCheckTx{
Tx: txBytes,
Type: abci.CheckTxType_New,
}
checkTxRes := s.baseApp.CheckTx(reqCheckTx)
s.True(checkTxRes.IsOK())

failTx := newTxCounter(s.txConfig, 1, 1)
failTx = setFailOnAnte(s.txConfig, failTx, true)
err = s.mempool.Insert(sdk.Context{}, failTx)
s.NoError(err)
s.Equal(2, s.mempool.CountTx())

req := abci.RequestPrepareProposal{
MaxTxBytes: 1000,
}
res := s.baseApp.PrepareProposal(req)
s.Equal(1, len(res.Txs))
}
Loading