OpenZeppelin/openzeppelin-subgraphs

Duplicate key value violates unique constraint after #25

ernestognw opened this issue · 15 comments

Description

After #25, some entities were moved to immutable: true, which is reasonable and shouldn't be a problem. However, there are some entities that are being saved twice (with the same data), so there was no issue previous to the migration to immutable, but now it flags them as duplicated.

Overall, this issue is not happening to all of the immutable entities, but here are some examples

Captura de Pantalla 2022-06-19 a la(s) 4 46 42

Captura de Pantalla 2022-06-19 a la(s) 8 05 00

Proposed solution

Just check that any immutable entity that already exists doesn't get saved again.

Amxx commented

However, there are some entities that are being saved twice (with the same data), so there was no issue previous to the migration to immutable, but now it flags them as duplicated.

Is that true? AFAIK, pushing the same entry with the same data twice was not a problem

Amxx commented

(closed by mistake)

Amxx commented

can you point to a subgraph instance that fails because of that ?

Amxx commented

I managed to replicate the issue. F***

Amxx commented

It apparently doesn't happen for ERC1155. Any reason why ?

Amxx commented

Also, with access control, it apparently isn't consistent. I tried indexing a contract I know, and I had no role_id_key issues

can you point to a subgraph instance that fails because of that ?

I can deploy one, but it'll take some time, and with the fix I sent, every subgraph that I deployed with these scripts worked, so I think that's consistent.

However, I have a concrete example of how it's failing:

  1. The Access Control subgraph for Rinkeby always failed on this block
  2. Taking a look at that, you can see there are two transactions creating contracts and emitting the RoleGranted event. This one and this one.
  3. This block was creating the duplicated role_id_key problem because it was creating an entry with the same role id

I can redeploy the failing subgraph to Rinkeby so you can take a look if needed.

Amxx commented

Thanks for these pointers

Amxx commented

This block was creating the duplicated role_id_key problem because it was creating an entry with the same role id

But then, any subgraph where the same role is granted multiple times (or granted then removed), should fail because of this line ... expect that doesn't happen.

Amxx commented

I started syncing a subgraph that indexes all accesscontrol on rinkeby starting at block 6757133, and it indexes the two contracts you identified :/

Edit: and then it fails at 6931053

But then, any subgraph where the same role is granted multiple times (or granted then removed), should fail because of this line ... expect that doesn't happen.

I didn't find any other deployed subgraph using the most recent immutable: true change, so although it's happening, there were no examples I guess.

I started syncing a subgraph that indexes all accesscontrol on rinkeby starting at block 6757133, and it indexes the two contracts you identified :/

Edit: and then it fails at 6931053

Yeah, if you start on the block I said seems like it doesn't fail because the collision happens with a block that's before.
I re-deployed the Rinkeby one here: https://thegraph.com/hosted-service/subgraph/ernestognw/access-control-rinkeby?version=pending but it's from block 0 and it's the only consistent way I know of making it fail.

Amxx commented

I wonder what that fails, but indexing a single contract (such as "0x41545f8b9472D758bB669ed8EaEEEcD7a9C4Ec29" on mainnet) does not cause the same error.

I got this answer

So for what I know, you can save an immutable entity multiple times in the block that creates it. It is only sealed after the block is fully processed.

But the instance above loads recreates the Role entity every time the role is granted on some contract, which did happen with 6000 blocks between events.

I wonder what that fails, but indexing a single contract (such as "0x41545f8b9472D758bB669ed8EaEEEcD7a9C4Ec29" on mainnet) does not cause the same error.

I got this answer

So for what I know, you can save an immutable entity multiple times in the block that creates it. It is only sealed after the block is fully processed.

Yeah, that's what I thought. If that's the case, but we still don't want to add a lot of .load, we should identify which entities are prone to this kind of error and guard it with a .load

But the instance above loads recreates the Role entity every time the role is granted on some contract, which did happen with 6000 blocks between events.

So fetchRole was called many times in between the contract creation and the error? WAS!?

btw, indeed it failed when I was expecting:
https://thegraph.com/hosted-service/subgraph/ernestognw/access-control-rinkeby?version=pending

Amxx commented

But the instance above loads recreates the Role entity every time the role is granted on some contract, which did happen with 6000 blocks between events.

So fetchRole was called many times in between the contract creation and the error? WAS!?

There is not even an error on this subgraph.

Amxx commented

It also happens with ERC1155!