Skip to content
This repository was archived by the owner on Jan 20, 2026. It is now read-only.

Add deferred withdrawals for module accounts#61

Merged
BrandonWeng merged 7 commits intomainfrom
bweng-deferred-helpers
Oct 24, 2022
Merged

Add deferred withdrawals for module accounts#61
BrandonWeng merged 7 commits intomainfrom
bweng-deferred-helpers

Conversation

@BrandonWeng
Copy link
Contributor

@BrandonWeng BrandonWeng commented Oct 20, 2022

Describe your changes and provide context

Refactors the code a bit and add necessary helpers for Tokenfactory mint and burn message types.

  • They both will deposit and withdraw from module accounts as they're used as intermediaries for storage

For context, module accounts are a single source of the bottom neck when it comes to concurrency, so to remove this bottle neck, we avoid declaring them in the access ops, instead, we store an in-memory value protected by mutex locks and flush it to the AVL tree at the very end. This will ensure deterministic updates

Follow-up PRs, still need to test concurrency:
sei-protocol/sei-chain#331
#63

Testing performed to validate your change

Mainly unit tests - will need to do some E2E testing once it's integrated with the sei-chain level chains. Will follow up with two more PRs

Sanity check that bank sends aren't affected with the follow up PRs:
image

@BrandonWeng BrandonWeng requested a review from udpatil October 20, 2022 05:58
@BrandonWeng BrandonWeng marked this pull request as ready for review October 20, 2022 05:58
@BrandonWeng BrandonWeng force-pushed the bweng-deferred-helpers branch from bd1304c to aeb5caa Compare October 20, 2022 22:02
Comment on lines -63 to -69

// Mapping of Module Account to the amount of tokens that have been deducted from
// Sender accounts but not yet deposited into module accounts. Use this to remove
// bottle neck for concurrent transacation processing and perform batch deposit
// in the end block
moduleAccountDepositMapping map[string]sdk.Coins
moduleAccountDepositMappingLock *sync.Mutex
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed

apply(recipientModule, c.deferredSends[recipientModule])
}
func (c *ContextMemCache) UpsertDeferredWithdrawals(moduleAccount string, amount Coins) {
// Separate locks needed for all mappings - atmoic transaction needed
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - typo atomic

@BrandonWeng BrandonWeng mentioned this pull request Oct 22, 2022
@BrandonWeng BrandonWeng merged commit e227788 into main Oct 24, 2022
@BrandonWeng BrandonWeng deleted the bweng-deferred-helpers branch October 24, 2022 14:43
BrandonWeng added a commit that referenced this pull request Oct 27, 2022
## Describe your changes and provide context
Refactors the code a bit and add necessary helpers for Tokenfactory mint
and burn message types.
* They both will deposit and withdraw from module accounts as they're
used as intermediaries for storage

For context, module accounts are a single source of the bottom neck when
it comes to concurrency, so to remove this bottle neck, we avoid
declaring them in the access ops, instead, we store an in-memory value
protected by mutex locks and flush it to the AVL tree at the very end.
This will ensure deterministic updates

Follow-up PRs, still need to test concurrency:
sei-protocol/sei-chain#331 
#63

## Testing performed to validate your change
Mainly unit tests - will need to do some E2E testing once it's
integrated with the sei-chain level chains. Will follow up with two more
PRs

Sanity check that bank sends aren't affected with the follow up PRs:

![image](https://user-images.githubusercontent.com/18161326/197229251-0afe0517-c930-4745-993a-10aad7c1c038.png)
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