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

Manage pending in memory and move it to synchroniser #1844

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

IronGauntlets
Copy link
Contributor

Fixes parts of #1789. A migration to remove the pending from DB is still outstanding.

It would be best to review the commits individually and in order.

@IronGauntlets IronGauntlets force-pushed the IronGauntlets/fix-pending-block-issues branch from 4d851b3 to cf6de15 Compare April 27, 2024 01:37
@IronGauntlets
Copy link
Contributor Author

This also fixes #1450

Copy link

codecov bot commented Apr 27, 2024

Codecov Report

Attention: Patch coverage is 68.50394% with 40 lines in your changes are missing coverage. Please review.

Project coverage is 74.98%. Comparing base (619fffd) to head (09e6778).

Files Patch % Lines
rpc/transaction.go 52.50% 14 Missing and 5 partials ⚠️
sync/sync.go 63.46% 15 Missing and 4 partials ⚠️
blockchain/event_filter.go 85.71% 0 Missing and 1 partial ⚠️
rpc/helpers.go 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1844      +/-   ##
==========================================
+ Coverage   74.92%   74.98%   +0.05%     
==========================================
  Files          96       96              
  Lines        8183     8114      -69     
==========================================
- Hits         6131     6084      -47     
+ Misses       1523     1513      -10     
+ Partials      529      517      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IronGauntlets
Copy link
Contributor Author

Since I will be away for 2 weeks. Please "Rebase and Merge" once it is approved.

@IronGauntlets IronGauntlets force-pushed the IronGauntlets/fix-pending-block-issues branch 2 times, most recently from 5e2d6e6 to cc6cf21 Compare May 4, 2024 16:45
Pending Block is now only managed in memory this is to make sure that
pending block in the DB and in memory do not become out of sync. Before
the pending block was managed in memory as a cache, however, since there
is only one pending block at a given time it doesn't make sense to keep
track of pending block in both memory and DB.

To reduce the number of block not found errors while simulating
transactions it was decided to store empty pending block, using the
latest header to fill in fields such as block number, parent block hash,
etc. This meant that any time we didn't have a pending block this
empty pending block would be served along with empty state diff and
classes. Every time a new block was added to the blockchain a new empty
pending block was also added to the DB.

The unforeseen side effect of this change was when the
--poll-pending-interval flag was disabled the rpc would still serve a
pending block. This is incorrect behaviour.

As the blocks changed per new versions of starknet the empty block also
needed to be changed and a storage diff with a special contract "0x1"
needed to be updated in the state diff. This overhead is unnecessary and
incorrectly informs the user that there is a pending block when
there isn't one.
In order to moved handling of pending to synchroniser the following
changes needed to be made:
- Add database to synchroniser, so that pending state can be served
- Blockchain and Events Filter have a pendingBlockFn() which returns the
  pending block. Due to import cycle pending struct could not be
  referenced, therefore, the anonymous function is passed.
- Add PendingBlock() to return just the pending block, this was mainly
  added to support the pendingBlockFn().
- In rpc package the pending block and state is retrieved through
  synchroniser. Therefore, receipt and transaction handler now check the
  pending block for the requested transaction/receipt.
@IronGauntlets IronGauntlets force-pushed the IronGauntlets/fix-pending-block-issues branch from cc6cf21 to 09e6778 Compare May 8, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant