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

autoOffset fails if the target pool's balance of the lowest scored TCO2 is zero #31

Open
danceratopz opened this issue Aug 9, 2022 · 4 comments
Assignees

Comments

@danceratopz
Copy link
Contributor

Since 0x7f91...604c in block 31487844 (Aug-03-2022 10:11:10 PM +UTC), the lowest scored TCO2 in the NCT pool (TCO2-VCS-1052-2012) has balance zero. This state was previously created on Jul-11-2022 08:11:25 PM +UTC in 0xb402...7573 but subsequently reverted. These transactions, with the subsequent deposit of the TCO2 to the BCT pool, are listed here.

If the array of scoredTCO2s in the NCT storage has not been (manually) updated to reflect the fact that the NCT pool's balance of the lowest scored TCO2 is zero the autoOffset* functions in the OffsetHelper fail:

  1. autoRedeem() returns arrays of TCO2 addresses and amounts that were redeemed as returned by autoRedeem2(). These arrays contain entries corresponding to the TCO2 with the lowest score from scoredTCO2s (even though the lowest scored TCO2 is not present in the NCT pool).
  2. autoRetire() then tries to retire tokens from the lowest scored TCO2, even though the amount redeemed was zero, and the transaction is reverted.
@pheuberger
Copy link

tagging @lazaralex98 to triage

danceratopz added a commit to danceratopz/toucan-example-implementations that referenced this issue Aug 9, 2022
This test case first creates the state described in the ticket as a setup step, ie, that the lowest scored TCO2 is not present in the NCT pool. Then it tests that autoRetire works, even if the lowest scored TCO2 is not in the pool.
danceratopz added a commit to danceratopz/toucan-example-implementations that referenced this issue Aug 9, 2022
@danceratopz
Copy link
Contributor Author

tagging @lazaralex98 to triage

Thanks Philipp, please see #32.

@mauricedesaxe
Copy link
Contributor

mauricedesaxe commented Aug 10, 2022

Indeed redeemAuto2 in the pool contracts returns the scoredTCO2s as is. Meaning, it checks internally which have balance 0 and only redeems the ones that have a balance, but returns all of them instead of a filtered version of the array.

Regardless of that, it seems to me like OffetHelper. autoRedeem() shouldn't fail because of it (?). redeemAuto2 does also return an amounts array, so in the OffsetHelper if a tco2 has 0 balance, it'd attempt to retire 0.

And, indeed if we look at the retire() method of the ToucanCarbonOffsets contract, I think it shouldn't fail if we try to retire 0.

EDIT: Obviously it's not efficient or good if it attempts to retire 0, but I don't think it should cause a failure.

@danceratopz
Copy link
Contributor Author

Indeed redeemAuto2 in the pool contracts returns the scoredTCO2s as is. Meaning, it checks internally which have balance 0 and only redeems the ones that have a balance, but returns all of them instead of a filtered version of the array.

Regardless of that, it seems to me like OffetHelper. autoRedeem() shouldn't fail because of it (?). redeemAuto2 does also return an amounts array, so in the OffsetHelper if a tco2 has 0 balance, it'd attempt to retire 0.

And, indeed if we look at the retire() method of the ToucanCarbonOffsets contract, I think it shouldn't fail if we try to retire 0.

EDIT: Obviously it's not efficient or good if it attempts to retire 0, but I don't think it should cause a failure.

I looked at this in more detail and tried to explain why the txn gets reverted here: ToucanProtocol/contracts/issues/5.

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

No branches or pull requests

3 participants