connext / monorepo

Connext is a modular stack for trust-minimized, generalized communication between blockchains.
https://docs.connext.network
MIT License
293 stars 173 forks source link

[ICE: 23] Allowance issue (call revert exception) #6107

Open oswy-cpu opened 7 months ago

oswy-cpu commented 7 months ago

Describe the bug image

Link to ticket in discord https://discord.com/channels/454734546869551114/1232897391233994774

Additional context A bunch of different users facing the same issue bridging ezETH from L1.

@oncall

SLA: 2 working days

oswy-cpu commented 7 months ago

A bunch of other related tickets:

jwtong commented 7 months ago

Is this a token allowance issue? Are they supposed to be able to bridge without setting an allowance here? Any info would help

ekbainova commented 7 months ago

New: https://discord.com/channels/454734546869551114/1233845569420525690 https://tickettool.xyz/transcript/v1/993555046891540570/1234511912658665472/transcript-closed-3127-verygood.html/66310050/662faed0/ccfadda21447b672f6faa772c225c5501377e88acf55d95e38511127f224ebff

ekbainova commented 6 months ago

I guess two more: https://discord.com/channels/454734546869551114/1236563765370294272 https://app.intercom.com/a/inbox/qge2vd8z/inbox/shared/all/conversation/670?view=List

ekbainova commented 6 months ago

2 tickets now in Intercom Continue collecting https://app.intercom.com/a/inbox/qge2vd8z/inbox/conversation/837?view=List

ekbainova commented 6 months ago

Now here is 6 tickets: https://app.intercom.com/a/inbox/qge2vd8z/inbox/conversation/837?view=List

regexpressyourself commented 6 months ago

@ekbainova I'm looking through Intercom. I see one user indicates they have 0 allowance, which I believe is why we're throwing an error when calling allowance(). Other users don't specify their allowance.

At this point, I'm assuming the allowance() call is failing when the user does not have the required allowance.

What should we do in this situation to resolve this issue?

ekbainova commented 6 months ago

@regexpressyourself what sort of allowance? I don't get it, sorry.

just-a-node commented 6 months ago

@regexpressyourself There's an sdk.approveIfNeeded call that should happen prior to xcall. The approveIfNeeded call checks if the asset is an xERC20. If so, it looks up the allowance on the LockboxAdapter otherwise it looks it up for the Connext contract. If there isn't sufficient allowance, the txn object returned by approveIfNeeded should be sent by the user first. Only after that should the txn object returned by xcall be sent to the user to send. I think we need to look into the sequencing of these events.

regexpressyourself commented 6 months ago

@regexpressyourself There's an sdk.approveIfNeeded call that should happen prior to xcall. The approveIfNeeded call checks if the asset is an xERC20. If so, it looks up the allowance on the LockboxAdapter otherwise it looks it up for the Connext contract. If there isn't sufficient allowance, the txn object returned by approveIfNeeded should be sent by the user first. Only after that should the txn object returned by xcall be sent to the user to send. I think we need to look into the sequencing of these events.

@just-a-node Got it.

The sequence appears correct when looking through the code and cross referencing the logs in the screenshot above. The approveIfNeeded call happens before the "set for an XERC20" logs, which we see in the screenshot logs. So it appears the sequencing there is ok.

Following the approveIfNeeded call, we "setup for an XERC20". The first step from there is a series of allowance() calls.

The failure here seems to occur on one of the calls to allowance() when working with an XERC20 asset. For XERC20 assets, we make 6 different allowance() calls at once. At least one of these calls is failing and returning the "call revert exception" error.

I've still been unable to reproduce this myself. When bridging XERC20 assets, I see the correct sequence of calls, and am able to approveIfNeeded and get through to the next txn:

image

Any more info on how to reproduce this, or some extra logging data would be super helpful in tracking this down. In the meantime, I've opened a PR with some additional logging that we can collect when reports of this come in: https://github.com/connext/frontend/pull/74

just-a-node commented 6 months ago

Merged logs PR

ekbainova commented 5 months ago

6272 - connected?