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.

Sign up for GitHub

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

Fix autoRetire when the lowest scored tco2 is not present in the pool #31 #32

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+91 −20
Draft

Fix autoRetire when the lowest scored tco2 is not present in the pool #31 #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
00b7d33
tests: add a test case for #31
danceratopz Aug 9, 2022
f0489cf
fix: don't call retire if the tco2 amount is zero #31
danceratopz Aug 9, 2022
d37cbd0
chore: enable github actions for pushes and prs on all branches
danceratopz Aug 9, 2022
eb19d3a
fix: increment counter in while loop, forgotten in f0489cfb
danceratopz Aug 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
name: Smart contracts

on:
push:
branches: [main]
pull_request:
branches: [main]
on: ["push", "pull_request"]

jobs:
build:
Expand Down
9 changes: 4 additions & 5 deletions contracts/OffsetHelper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,10 @@ contract OffsetHelper is OffsetHelperStorage {
balances[msg.sender][_tco2s[i]] >= _amounts[i],
"Insufficient TCO2 balance"
);

balances[msg.sender][_tco2s[i]] -= _amounts[i];

IToucanCarbonOffsets(_tco2s[i]).retire(_amounts[i]);

if (_amounts[i] > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a fix for this deployed in Mumbai for both BCT and NCT, you don't need this change anymore. Btw @danceratopz it would be great if you could confirm that the issue is fixed for you now!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Kargakis, it's not possible to verify whether the issue has been fixed on Polygon Mainnet with the current chain state as there are plenty of the lowest scored TCO2, TCO2-VCS-1052-2012, in the NCT Pool, https://polygonscan.com/token/0x463de2a5c6e8bb0c87f4aa80a02689e6680f72c7#balances.

As the issue only reveals itself when the lowest scored TCO2 is not present in the pool, it's probably not possible to test this issue on Mumbai either, but I didn't check (I was a bit lazy as the ABI for NCT on Mumbai has not been published https://mumbai.polygonscan.com/address/0x7beCBA11618Ca63Ead5605DE235f6dD3b25c530E#readContract).

Btw, I don't think there are liquidity pools for NCT or BCT setup on Sushiswap on Mumbai, so we never extensively used or tested the OffsetHelper there, cf https://discord.com/channels/706114892666765375/929319519430799360/1014126215877103647

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danceratopz good point on liquidity pool setup in Mumbai, @lazaralex98 this is something we need to have in place in order for the offsetter contract to be prod-ready, do we track TODOs for that anywhere atm?

The first TCO2 in the score array in BCT is empty, we can test this issue by redeeming BCT: https://mumbai.polygonscan.com/address/0xf2438A14f668b1bbA53408346288f3d7C71c10a1#readProxyContract
The offsetter contract btw should expose all the functions needed to offset without the need to swap so the lack of LPs in Mumbai should not block testing of this issue.

Copy link
Contributor Author

@danceratopz danceratopz Aug 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kargakis Sorry, Michalis, I didn't check BCT on Mumbai. Yes, I can confirm the fix, the (original) Mumbai OffsetHelper now takes the second TCO2 in the list of scored TCO2s:
https://mumbai.polygonscan.com/tx/0xfbe23052dabc36e070d2b2b313ac9883159d3c6f2616c9ba21d0df91f6986cfb

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danceratopz

I was a bit lazy as the ABI for NCT on Mumbai has not been published

It has been published, but it's an upgradeable contract so you have to look at read as proxy.

I don't think there are liquidity pools for NCT or BCT setup on Sushiswap on Mumbai

We indeed do not have that, nor is this planned anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point on liquidity pool setup in Mumbai, @lazaralex98 this is something we need to have in place in order for the offsetter contract to be prod-ready, do we track TODOs for that anywhere atm?

We do have a Notion proposal with steps documented to take the OffsetHelper production ready. I'll add bringing LPs on Mumbai as a step & share it with you on Slack.

balances[msg.sender][_tco2s[i]] -= _amounts[i];
IToucanCarbonOffsets(_tco2s[i]).retire(_amounts[i]);
}
unchecked {
++i;
}
Expand Down
85 changes: 76 additions & 9 deletions test/OffsetHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ import {

import * as hardhatContracts from "../utils/toucanContracts.json";
import * as poolContract from "../artifacts/contracts/interfaces/IToucanPoolToken.sol/IToucanPoolToken.json";
import * as carbonOffsetsContract from "../artifacts/contracts/interfaces/IToucanCarbonOffsets.sol/IToucanCarbonOffsets.json";
import {
IToucanPoolToken,
OffsetHelper,
OffsetHelper__factory,
Swapper,
Swapper__factory,
} from "../typechain";
import addresses from "../utils/addresses";
import addresses, { whaleAddresses } from "../utils/addresses";
import { Contract } from "ethers";
import { usdcABI, wethABI, wmaticABI } from "../utils/ABIs";

Expand Down Expand Up @@ -85,13 +86,31 @@ describe("Offset Helper - autoOffset", function () {
]
);

await Promise.all(
addrs.map(async (addr) => {
await addr.sendTransaction({
to: addr2.address,
value: (await addr.getBalance()).sub(parseEther("1.0")),
});
})
// Transfer a large amount of MATIC and NCT to the test account via account impersonation.
mauricedesaxe marked this conversation as resolved.
Show resolved Hide resolved
await network.provider.request({
method: "hardhat_impersonateAccount",
params: [whaleAddresses.matic],
});
const maticWhale = ethers.provider.getSigner(whaleAddresses.matic);
await maticWhale.sendTransaction({
to: addr2.address,
value: (await maticWhale.getBalance()).sub(parseEther("1.0")),
});

// Note: The swapper fails when trying to exchange such a large amount of NCT.
await network.provider.request({
method: "hardhat_impersonateAccount",
params: [whaleAddresses.nct],
});
const addrNctWhale = ethers.provider.getSigner(whaleAddresses.nct);
const nctWhaleSigner = new ethers.Contract(
addresses.nct,
hardhatContracts.contracts.NatureCarbonTonne.abi,
addrNctWhale
);
await nctWhaleSigner.transfer(
addr2.address,
await nctWhaleSigner.balanceOf(whaleAddresses.nct)
);

await swapper.swap(addresses.weth, parseEther("20.0"), {
Expand Down Expand Up @@ -298,7 +317,7 @@ describe("Offset Helper - autoOffset", function () {
).to.be.revertedWith("Insufficient TCO2 balance");
});

it("Should retire using an NCT deposit", async function () {
it("Should retire using a BCT deposit", async function () {
danceratopz marked this conversation as resolved.
Show resolved Hide resolved
await (await bct.approve(offsetHelper.address, parseEther("1.0"))).wait();

await (
Expand All @@ -319,6 +338,54 @@ describe("Offset Helper - autoOffset", function () {

await expect(offsetHelper.autoRetire(tco2s, amounts)).to.not.be.reverted;
});

it("Should retire using an NCT deposit, even if the first scored TCO2 is not in pool", async function () {
const scoredTCO2s = await nct.getScoredTCO2s();
const lowestScoredTCO2 = new ethers.Contract(
scoredTCO2s[0],
carbonOffsetsContract.abi,
addr2
);
const lowestScoredTCO2Balance = await lowestScoredTCO2.balanceOf(
addresses.nct
);

// Skip setup if the oldest tco2's balance in the pool is already 0.
if (formatEther(lowestScoredTCO2Balance) !== "0.0") {
// Setup: If the oldest tco2 balance is non-zero, remove all its tokens from the pool via a redeem.
mauricedesaxe marked this conversation as resolved.
Show resolved Hide resolved
// Ensure that addr2 has enough NCT to redeem all of the lowestScoredTCO2 or setup will fail.
mauricedesaxe marked this conversation as resolved.
Show resolved Hide resolved
expect(await nct.balanceOf(addr2.address)).to.be.above(
await lowestScoredTCO2.balanceOf(addresses.nct)
);

await nct.approve(offsetHelper.address, lowestScoredTCO2Balance);

await offsetHelper.deposit(addresses.nct, lowestScoredTCO2Balance);

await offsetHelper.autoRedeem(addresses.nct, lowestScoredTCO2Balance);
}

// Ensure the test condition is met.
expect(await lowestScoredTCO2.balanceOf(addresses.nct)).to.equal(0);

await nct.approve(offsetHelper.address, parseEther("1.0"));

await offsetHelper.deposit(addresses.nct, parseEther("0.0005"));

const redeemReceipt = await (
await offsetHelper.autoRedeem(addresses.nct, parseEther("0.0005"))
).wait();

if (!redeemReceipt.events) {
return;
}
const tco2s =
redeemReceipt.events[redeemReceipt.events.length - 1].args?.tco2s;
const amounts =
redeemReceipt.events[redeemReceipt.events.length - 1].args?.amounts;

await expect(offsetHelper.autoRetire(tco2s, amounts)).to.not.be.reverted;
});
});

describe("Testing deposit() and withdraw()", function () {
Expand Down
11 changes: 10 additions & 1 deletion utils/addresses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ interface IfcAddresses {
wmatic: string;
}

const addresses: IfcAddresses = {
interface IfcWhaleAddresses {
matic: string;
nct: string;
}
export const addresses: IfcAddresses = {
myAddress: "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266",
bct: "0x2F800Db0fdb5223b3C3f354886d907A671414A7F",
nct: "0xD838290e877E0188a4A44700463419ED96c16107",
Expand All @@ -16,6 +20,11 @@ const addresses: IfcAddresses = {
wmatic: "0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270",
};

export const whaleAddresses: IfcWhaleAddresses = {
matic: "0xe7804c37c13166fF0b37F5aE0BB07A3aEbb6e245",
nct: "0x4b3ebae392e8b90a9b13068e90b27d9c41abc3c8",
};

export const mumbaiAddresses: IfcAddresses = {
myAddress: "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266",
bct: "0xf2438A14f668b1bbA53408346288f3d7C71c10a1",
Expand Down