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.

Sign up for GitHub

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

Jump to bottom

feat: ope poc #203

Closed
wants to merge 18 commits into from
Closed

feat: ope poc #203

wants to merge 18 commits into from

Conversation

dudong2
Copy link

@dudong2 dudong2 commented Oct 17, 2024

Description

Closes: #XXXX

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

dudong2 and others added 11 commits October 16, 2024 11:34
@dudong2
chore: Change go.mod for local develop
31d01df
@yihuang @dudong2
Problem: parallel tx execution is not supported (#205)
ccfb095 
add basic support in sdk:
- add a TxExecutor baseapp option
- add TxIndex/TxCount/MsgIndex in context

Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

fix misspell

fix lint

run gci

fix lint

gci seems not compatible with gofumpt
@yihuang @dudong2
Support object store (#206)
fa643d0 
generic interface

generic btree

generic cachekv

generic transient store

support ObjStore

changelog

Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

object store key

Apply review suggestions

fix merge conflict

fix snapshot

revert dependers
@yihuang @dudong2
Problem: MultiStore interface is bloated (#240)
a9383c4 
* Problem: MultiStore interface is bloated

Solution:
- Split out specialied methods from it, keeping the MultiStore generic

* Update store/CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

---------

Signed-off-by: yihuang <[email protected]>
@yihuang @dudong2
Problem: no API to use the new CoW branch store (#243)
ea42a34 
* Support RunAtomic API

* add unit test
@yihuang @dudong2
Problem: fee collection not compatible with parallel execution (#237)
f1220f3 
* Problem: no efficient way to collect fee

Solution:
- support an idea of virtual account in bank module, where the incoming
  coins are accumulated in a per-tx object store first, then accumulate
  and credit to the real account at end blocker.

  it's nesserary to support parallel tx execution, where we try not to
  access shared states.

more efficient sum

support SendCoinsFromModuleToAccountVirtual

fix test

fix test

* fix lint

* fix test

* fix test

* fix test

* fix test

* fix test

* fix mock keeper

* try fix lint

* try fix lint

* reuse code

* try fix linter

* Update x/bank/keeper/send.go

Signed-off-by: yihuang <[email protected]>

* algin panic call

* fix error handling

* try fix lint

* nolintlint generate falst postiive

---------

Signed-off-by: yihuang <[email protected]>
@yihuang @dudong2
prefix store support object store (#236)
2ea453c
@yihuang @dudong2
Problem: signature verification result not cache between incarnations…
735d2f6 
… of same tx (cosmos#565)

* Problem: signature verification result not cache between incarnations of
same tx

Closes: cosmos#564

Solution:
- introduce incarnation cache that's shared between incarnations of the
  same tx

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* Update types/context.go

Signed-off-by: yihuang <[email protected]>

* Update types/context.go

Signed-off-by: yihuang <[email protected]>

* fix nil convert

---------

Signed-off-by: yihuang <[email protected]>
@yihuang @dudong2
Problem: tx executor can't do dependency analysis (cosmos#744)
8b94fa6 
* Problem: tx executor can't do dependency analysis

Solution:
- change the api to allow static analysis on tx body

* fix

* changelog

* cleanup

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

---------

Signed-off-by: yihuang <[email protected]>
@dudong2
chore: Fix go.mod
9c3fb3b
@dudong2
chore: Apply github url for cosmossdk.io/store
2b7fae2
@dudong2 dudong2 self-assigned this Oct 17, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 17, 2024
View reviewed changes
testutil/integration/router.go
Comment on lines +188 to +190
for key := range okeys {
cms.MountStoreWithDB(okeys[key], storetypes.StoreTypeObject, nil)
}

Check warning

Code scanning / CodeQL

Iteration over map Warning test

Iteration over map may be a possible source of non-determinism
x/bank/module.go
@@ -193,6 +195,10 @@
)
}

func (am AppModule) EndBlock(ctx context.Context) error {
return am.keeper.CreditVirtualAccounts(ctx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
Copy link
Member

Choose a reason for hiding this comment

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

Could you provide reference to a related Cronos PR for this part? It seems necessary to review whether virtual bank functions in other modules were missed and should be considered together.

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 21, 2024
View reviewed changes
baseapp/reactor.go
}

req.callback(&abci.ResponseCheckTx{
GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?

Check failure

Code scanning / gosec

integer overflow conversion uint64 -> uint8 Error

integer overflow conversion uint64 -> int64
baseapp/reactor.go

req.callback(&abci.ResponseCheckTx{
GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints?
GasUsed: int64(gInfo.GasUsed), // TODO: Should type accept unsigned ints?

Check failure

Code scanning / gosec

integer overflow conversion uint64 -> uint8 Error

integer overflow conversion uint64 -> int64
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 21, 2024
View reviewed changes
baseapp/abci.go
}
app.chCheckTx <- reqCheckTx

go app.prepareCheckTx(reqCheckTx)

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
baseapp/reactor.go
)

func (app *BaseApp) startReactors() {
go app.checkTxAsyncReactor()

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
baseapp/reactor.go

waits, signals := app.checkAccountWGs.Register(app.cdc, req.tx)

go app.checkTxAsync(req, waits, signals)

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Nov 21, 2024
@github-actions github-actions bot closed this Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongsam dongsam dongsam left review comments

Assignees

@dudong2 dudong2

Labels
C:x/auth C:x/bank C:x/gov C:x/group Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dudong2 @dongsam @yihuang