Create Channels from DAG by BrandonWeng · Pull Request #261 · sei-protocol/sei-chain

BrandonWeng · 2022-09-26T09:34:41Z

Creates channels based on the DAG

Pointing the PR to #258

Tested by running: ./scripts/old_initialize_local.sh

BrandonWeng · 2022-09-26T09:34:52Z

app/app.go

+}
+
+// Returns a mapping of the access index to the channels
+func getChannelsFromSignalMapping(signalMapping map[acltypes.AccessOperation][]CompletionSignal) (channelsMapping [][](chan interface{})){


PR to update Context object sei-protocol/sei-cosmos#27

BrandonWeng · 2022-09-26T09:35:49Z

app/graph.go

 			FromNodeID:      edge.FromNodeID,
 			ToNodeID:        edge.ToNodeID,
 			AccessOperation: *edge.AccessOperation,
+			Channel:		 make(chan interface{}),


Thought it would make sense to store the channel here as it's set for both to and from nodes

makes sense to me 👍

udpatil · 2022-09-26T16:42:10Z

app/graph.go

 			FromNodeID:      edge.FromNodeID,
 			ToNodeID:        edge.ToNodeID,
 			AccessOperation: *edge.AccessOperation,
+			Channel:		 make(chan interface{}),


makes sense to me 👍

udpatil · 2022-09-26T17:58:21Z

app/app.go

+
+	// Gather Results
+	txResults := []*abci.ExecTxResult{}
+	for result := range resultChan {


this isnt necessarily going to be in the same order as txs that were provided, right? is that safe, or should we also include the tx index with the results so we can sort into the tx index order prior to returning Tx results?

Yes, this will be in the order that it's completed in - let me double check if the ordering matters

udpatil · 2022-09-26T18:05:47Z

app/app.go

+	// Store the Channels in the Context Object for each transaction
+	ctx.WithTxBlockingChannels(getChannelsFromSignalMapping(txBlockingSignalsMap))
+	ctx.WithTxCompletionChannels(getChannelsFromSignalMapping(txCompletionSignalingMap))
+


can we add TODOs to block on blocking channels and writing to completion channels to properly block and release resources

added here: https://github.com/sei-protocol/sei-chain/pull/261/files#diff-0f1d2976054440336a576d47a44a37b80cdf6701dd9113012bce0e3c425819b7R968

BrandonWeng · 2022-09-27T14:57:30Z

go.mod


 replace (
-	github.com/cosmos/cosmos-sdk => github.com/sei-protocol/sei-cosmos v0.1.67
+	github.com/cosmos/cosmos-sdk => ../sei-cosmos // github.com/sei-protocol/sei-cosmos v0.1.20


Will revert this once the sei-cosmos PR merges and I can bump up the version

BrandonWeng · 2022-09-27T14:57:46Z

app/graph.go

 	return
 }

-type CompletionSignal struct {


Moving this up with all the other types

philipsu522 · 2022-09-27T22:04:17Z

app/app.go

-				Events:    deliverTxResp.Events,
-				Codespace: deliverTxResp.Codespace,
-			})
+		if err == ErrCycleInDAG {


would be good to add metrics over how many times we process synchronously vs concurrently

I'll add a todo here, looks like we don't have the telemetry lib from master. It would be easier to do once we merge this into master

philipsu522 · 2022-09-27T22:08:48Z

app/app.go

+
+	// Waits for all the transactions to complete
+	go func() {
+		waitGroup.Wait()


we're waiting for the WG in a separate goroutine? doesn't that mean it won't block?

ah shoot, yeah good catch. We'll need another waitGroup.Wait() in the method too

ah what i meant is do we need this Wait()? From what I can tell, you essentially want to launch a bunch of goroutines in ln 1000 from this main thread, and block when it's finally all done. If that's the case, then the waitGroup.Wati() in ln 1018 is sufficient to achieve this

Ahh yeah, it's technically not really needed but more so it's just closing the channel. I guess in our case since we don't really need need to communicate that the data is done then we could just remove this

philipsu522 · 2022-09-27T22:09:23Z

app/app.go

+	// For each transaction, start goroutine and deliver TX
+	for txIndex, txBytes := range txs {
+		waitGroup.Add(1)
+		app.ProcessTxConcurrent(


dont we want this in a separate goroutine?

good catch 😵‍💫

philipsu522 · 2022-09-28T04:20:30Z

app/app.go

+
+	// Waits for all the transactions to complete
+	go func() {
+		waitGroup.Wait()


ah what i meant is do we need this Wait()? From what I can tell, you essentially want to launch a bunch of goroutines in ln 1000 from this main thread, and block when it's finally all done. If that's the case, then the waitGroup.Wati() in ln 1018 is sufficient to achieve this

app/app.go

* Create Channels from DAG * lint

This reverts commit 35b9cf4.

* Revert "Update Antehandler resource depedencies (#354)" This reverts commit 6c672fb. * Revert "update staking validation identifier (#353)" This reverts commit 8a17c26. * Revert "Update Oracle Parallel Tx Resource Identifiers (#351)" This reverts commit c9229ab. * Revert "Update IdentifierTemplate for TokenFactory (#350)" This reverts commit e81fc93. * Revert "[loadtest] Loadtest fixes" This reverts commit 68bfab5. * Revert "fix unit tests" This reverts commit 49eb809. * Revert "Fix Bank Send Unit Tests (#348)" This reverts commit 9c11837. * Revert "integrate with sei-cosmos changes (#347)" This reverts commit e1b1790. * Revert "Validate Concurrent Messages + Update BankSend (#345)" This reverts commit e4d3176. * Revert "Add concurrency for BurnMsg and MintMsg (#331)" This reverts commit 4a5cff0. * Revert "Ezhu/granularize staking resources (#343)" This reverts commit 90e288c. * Revert "Oracle parallel (#334)" This reverts commit 67c0db3. * Revert "Ezhu/staking dep gen mappings v2 rebase (#338)" This reverts commit 1324183. * Revert "Refactor Loadtest script (#336)" This reverts commit c8b83c7. * Revert "Optimize genesis account creation script (#320)" This reverts commit a0c0306. * Revert "Wasm gov (#316)" This reverts commit c3b421b. * Revert "Lazy Deposit All Module Accounts During EndBlock (#313)" This reverts commit 5ad4d9b. * Revert "Wasm signal (#305)" This reverts commit 206f2bb. * Revert "Add dependencies for ante handlers that read/write accounts (#314)" This reverts commit 5d39f90. * Revert "Add Gasless decorator back and remove CountTxDecorator (#311)" This reverts commit 93fa4bd. * Revert "Branch another cache for all transactions (#309)" This reverts commit c2ce1ed. * Revert "Add msg send dynamic access ops (#303)" This reverts commit 2c5f14e. * Revert "[ante] Add ante dep generator default behavior (#294)" This reverts commit 0ada1db. * Revert "Add more parallel TX metrics (#296)" This reverts commit 567ffd9. * Revert "Add aclmapping options and mapping folder (#287)" This reverts commit c9522a5. * Revert "Bump sei-cosmos and sei-tendermint for 2.0.0beta (#293)" This reverts commit d9254b6. * Revert "Cherry-pick loadtesting changes and make a fix for parallel tx (#288)" This reverts commit 255fec9. * Revert "[app] refactored graph into acl module (#286)" This reverts commit 9b2d204. * Revert "Add gov proposal handler for acl (#277)" This reverts commit bbb5a50. * Revert "Fix for DAG building switch cases (#282)" This reverts commit 9eebc8e. * Revert "Optimize signals (#280)" This reverts commit fd9a3b6. * Revert "[app] Add behavior to process blocks with gov txs sync (#276)" This reverts commit ea2a906. * Revert "[graph] Move metric to dag builder helper (#275)" This reverts commit c530c02. * Revert "Fixes for parallel TX and metrics (#272)" This reverts commit 30207e7. * Revert "[graph] Add resource hierarchy to dependency graph (#268)" This reverts commit da7c070. * Revert "Revert "Parallel TX Synchrnous Testing"" This reverts commit f343740. * Revert "Parallel TX Synchrnous Testing" This reverts commit b95b44c. * Revert "Dag optimization (#263)" This reverts commit ac3fdbf. * Revert "Create Channels from DAG (#261)" This reverts commit 35b9cf4. * Revert "[accesscontrol] Add dependency dag (#258)" This reverts commit 6721845. * Revert "Register accesscontrol module (#257)" This reverts commit 3e34d34.

## Describe your changes and provide context This replaces the original Upsert in the deferred send from module to account with one that doesn't perform a subtraction against the deferred sends. The callsites were also modified to ensure this deferred withdrawal would ONLY be called IFF we've already tried subtracting from the deferred sends with insuffucient balance AND there is sufficient bank balance. ## Testing performed to validate your change Tests in sei-chain verifying this behavior for known problematic scenarios.

BrandonWeng commented Sep 26, 2022

View reviewed changes

BrandonWeng requested a review from udpatil September 26, 2022 09:36

BrandonWeng assigned udpatil Sep 26, 2022

BrandonWeng mentioned this pull request Sep 26, 2022

Add Tx channels to context sei-protocol/sei-cosmos#27

Merged

BrandonWeng requested a review from codchen September 26, 2022 16:38

BrandonWeng assigned codchen Sep 26, 2022

udpatil reviewed Sep 26, 2022

View reviewed changes

BrandonWeng force-pushed the bweng-parallel-tx-channels branch from 6fb2273 to 696d40c Compare September 27, 2022 06:29

BrandonWeng requested a review from udpatil September 27, 2022 07:46

BrandonWeng force-pushed the bweng-parallel-tx-channels branch from 0abefd6 to b5f8a38 Compare September 27, 2022 07:54

BrandonWeng commented Sep 27, 2022

View reviewed changes

app/graph.go

return

}

type CompletionSignal struct {

Copy link

Contributor Author

BrandonWeng Sep 27, 2022

Choose a reason for hiding this comment

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

Moving this up with all the other types

Base automatically changed from dependency-dag to 2.0.0beta September 27, 2022 16:09

BrandonWeng requested a review from philipsu522 September 27, 2022 17:49

BrandonWeng assigned philipsu522 Sep 27, 2022

BrandonWeng force-pushed the bweng-parallel-tx-channels branch from e348010 to 569c6fc Compare September 27, 2022 18:50

philipsu522 reviewed Sep 27, 2022

View reviewed changes

udpatil approved these changes Sep 27, 2022

View reviewed changes

BrandonWeng requested a review from philipsu522 September 27, 2022 22:55

philipsu522 reviewed Sep 28, 2022

View reviewed changes

BrandonWeng requested a review from philipsu522 September 29, 2022 04:29

philipsu522 approved these changes Sep 29, 2022

View reviewed changes

BrandonWeng force-pushed the bweng-parallel-tx-channels branch from e3ff08b to a192f47 Compare September 29, 2022 15:59

Create Channels from DAG

da941f6

BrandonWeng force-pushed the bweng-parallel-tx-channels branch from 2dde61f to da941f6 Compare September 29, 2022 16:01

lint

3916a00

BrandonWeng merged commit eaf6272 into 2.0.0beta Sep 29, 2022

BrandonWeng deleted the bweng-parallel-tx-channels branch September 29, 2022 16:07

philipsu522 pushed a commit that referenced this pull request Sep 29, 2022

Create Channels from DAG (#261)

d79fcad

* Create Channels from DAG * lint

philipsu522 pushed a commit that referenced this pull request Oct 17, 2022

Create Channels from DAG (#261)

b1a58f9

* Create Channels from DAG * lint

udpatil pushed a commit that referenced this pull request Oct 27, 2022

Create Channels from DAG (#261)

030bb2e

* Create Channels from DAG * lint

udpatil pushed a commit that referenced this pull request Oct 27, 2022

Create Channels from DAG (#261)

7839a44

* Create Channels from DAG * lint

udpatil pushed a commit that referenced this pull request Oct 28, 2022

Create Channels from DAG (#261)

7ebb7e1

* Create Channels from DAG * lint

udpatil pushed a commit that referenced this pull request Nov 1, 2022

Create Channels from DAG (#261)

5b6aac9

* Create Channels from DAG * lint

udpatil pushed a commit that referenced this pull request Nov 1, 2022

Create Channels from DAG (#261)

35b9cf4

* Create Channels from DAG * lint

udpatil added a commit that referenced this pull request Nov 1, 2022

Revert "Create Channels from DAG (#261)"

1ad2ffe

This reverts commit 35b9cf4.

Conversation

BrandonWeng commented Sep 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BrandonWeng Sep 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrandonWeng Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BrandonWeng commented Sep 26, 2022 •

edited

Loading

BrandonWeng Sep 26, 2022 •

edited

Loading

BrandonWeng Sep 28, 2022 •

edited

Loading