fix: incorrect blocknumber in syncTaggedLogs#13152
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
benesjan
force-pushed
the
03-28-fix_incorrect_blocknumber_in_synctaggedlogs
branch
2 times, most recently
from
55959ec to
17768ae
Compare
nventuro
left a comment
nventuro
left a comment
There was a problem hiding this comment.
In general we'll need to be much better re. testing from now on, and given the current state of it is in general so poor we need to at least not muddle it further.
|
|
||
| // PXEOracleInterface.syncTaggedLogs(...) function syncs logs up to the block number up to which PXE synced. We set | ||
| // as sufficiently high sync number here to not interfere with the tests. | ||
| await setSyncedBlockNumber(100); |
There was a problem hiding this comment.
This seems very fragile. Why do we even need to do this in the setup?
There was a problem hiding this comment.
Because we need the PXE to be synced to a block larger than the largest block in any of the logs such that we get delivered all the logs by default in the tests.
There was a problem hiding this comment.
Makes sense to define the constants. Did that in fe43da9
There was a problem hiding this comment.
But why do it so early? We've not even created the PXE yet. And how do we know that block 3 is enough?
There was a problem hiding this comment.
But why do it so early?
No reason. Moved it to the end of the beforeEach function as I agree it makes more sense there.
And how do we know that block 3 is enough?
See the comment above the call to setSyncedBlockNumber. That block corresponds to the block of the last emitted log (as set by generateMockLogs function).
yarn-project/pxe/src/pxe_oracle_interface/pxe_oracle_interface.test.ts
Outdated
Show resolved
Hide resolved
benesjan
force-pushed
the
03-28-fix_incorrect_blocknumber_in_synctaggedlogs
branch
from
702806c to
fe43da9
Compare
benesjan
force-pushed
the
03-28-fix_incorrect_blocknumber_in_synctaggedlogs
branch
from
fe43da9 to
9fca6ed
Compare
benesjan
requested review from
nventuro
and removed request for
Thunkar and
nventuro
2 participants
In syncTaggedLog we used the block number from AztecNode instead of the one up to which PXE was synced.