Check grafting from pre 0.0.6 to 0.0.6 and later#5929
Conversation
zorancv
force-pushed
the
zoran/poi-test
branch
19 times, most recently
from
5a462d9 to
d215fc8
Compare
lutter
left a comment
lutter
left a comment
There was a problem hiding this comment.
Nice test! Just a few minor comments
tests/tests/integration_tests.rs
Outdated
| } | ||
| } | ||
| } | ||
| let one_sec = time::Duration::from_secs(1); |
There was a problem hiding this comment.
This would be better as a constant
tests/tests/integration_tests.rs
Outdated
| } | ||
| } | ||
| let one_sec = time::Duration::from_secs(1); | ||
| thread::sleep(one_sec); |
There was a problem hiding this comment.
Probably not a big deal here, but you shouldn't use thread::sleep in async code; use tokio::time::sleep instead.
There was a problem hiding this comment.
For current usage is irrelevant as all tests need to wait, but if ever used elsewhere it could matter. Changing it.
tests/tests/integration_tests.rs
Outdated
| } | ||
|
|
||
| async fn test_subgraph_grafting(ctx: TestContext) -> anyhow::Result<()> { | ||
| async fn get_bloock_hash(block_number: i32) -> Option<String> { |
| // We need to make sure that the preconditions for POI are fulfiled | ||
| // namely that the anvil produces the proper block hashes for the | ||
| // blocks of which we will check the POI | ||
| assert_eq!(block_hash, block_hashes[(i - 1) as usize]); |
There was a problem hiding this comment.
I am confused by this - the comment made me think we would check anvil, but we are querying graph-node. Maybe the comment should just not mention anvil
There was a problem hiding this comment.
Right. The 'blockchain' is more appropriate here,
| @@ -0,0 +1,25 @@ | |||
| specVersion: 0.0.5 | |||
| description: Grafted Subgraph | |||
There was a problem hiding this comment.
I find this naming confusing - 'grafted subgraph' implies that this subgraph is grafted onto something else. Maybe just calling it 'base subgraph' would be better. And what's currently called 'grafting subgraph' could then be called 'grafted subgraph', but leaving that as 'grafting subgraph' is fine.
There was a problem hiding this comment.
Agree, those names are more intuitive.
2 participants
This PR tries to check the consistency of the POI implementation for grafting of subgraph in case there is a transition from a spec version older than 0.0.6 towards the 0.0.6 or newer. The grafting block has a conversion of the legacy hashing algorithm to the new one, and here we test that this conversion did not change.
On order to keep the POI with the expected values, we need to make sure that anvil produces the same genesis block hence a fixed timestamp for starting it. Also both local start and remote one need to have same parameters. In addition to that we need to wait until the last checked grafted block is mined before we start the integration tests.
Moreover there is need for specifying an
indexparameter in the fetching POI query in order to avoid error when a local environment variableGRAPH_POI_ACCESS_TOKENis set as that would set it to zero address and produce inconsistent POI value.