-
Notifications
You must be signed in to change notification settings - Fork 262
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
Optimize safe events indexer #2194
Conversation
081c5e8
to
2140074
Compare
# ProxyCreation first and SafeSetup later | ||
proxy_creation_event = events[1] | ||
else: | ||
# Proxy was created in previous blocks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous implementation always tried to remove the InternalTx
generated from ProxyCreation
just in case the ProxyCreation
came first. https://github.com/safe-global/safe-transaction-service/blob/main/safe_transaction_service/history/indexers/safe_events_indexer.py#L366
In such case, because the ProxyCreation
never comes later, the InternalTx
generated by SafeSetup
leaves without singleton (to) forever.
Maybe would be better try to get the Proxy InternalTx from database to insert the correct parameters of SafeSetup
what do you think? @safe-global/core-api
In the current implementation the ProxyCreation
is storing the singleton address in the to
field of InternalTx
, not sure if is completely right.
safe_transaction_service/history/indexers/proxy_factory_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
InternalTx.objects.bulk_create(internal_txs) | ||
InternalTxDecoded.objects.bulk_create(internal_decoded_txs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if they are already inserted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raise an exception.
Ignoring conflicts in such case would fix the issue, 5eeabd4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Uxio0 ignore_conflicts
fail because InternalDecodedTx
have relation with internal_tx
, basically the ORM try to prevent lose data.
We can use instead of that update_conflicts, but I'm concerned because I just discovered that bulk_create does not call save, and in that case post_save
will not be fired.
Will this affect our signals handle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bulk_create
does in case we inherit our BulkCreateMixin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a workaround in case that would be a collision inserting the same instance, shouldn't be any common because the bulk create is in atomic section and we are checking if the contract were indexed, basically if the setup
and proxyevent
were correctly inserted, shouldn't reach the insert section if would be indexed again.
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Uxío <[email protected]>
Add test to check the case
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
safe_transaction_service/history/indexers/safe_events_indexer.py
Outdated
Show resolved
Hide resolved
Increase the test cases
Co-authored-by: Uxío <[email protected]>
Co-authored-by: Uxío <[email protected]>
Closes #2181
Description
This PR reduce the number of queries used to process the
SafeSetup
andProxyCreation
events, taking the advantage of commonly this two events are together in the same transaction or close in a batch of blocks.Is important to highlight if the first event is
SafeSetup
will be always together with aProxyCreation
event because is a result of callDeployProxy
with initializer.Benchmark
The following sctipt was used to compare the results between the previous implementation and the implementation of this PR.
Also used the same chain and RPC in both chains.
Results
Previous implementation:
Current implementation: