Skip to content

Commit

Permalink
"Max Buttons" incorrectly calculate max amounts to Redeem and Unwrap-…
Browse files Browse the repository at this point in the history
…Weth (#117)

* avoid casting position stats to numbers prematurely; redeem should set max to the pre-casted string

* Send txn with string not number
  • Loading branch information
nicholaspai authored Aug 11, 2020
1 parent 6150475 commit f2922a2
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 53 deletions.
24 changes: 12 additions & 12 deletions containers/Position.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@ import Collateral from "./Collateral";
import Token from "./Token";

const fromWei = ethers.utils.formatUnits;
const weiToNum = (x: BigNumberish, u = 18) => parseFloat(fromWei(x, u));
const weiToNum = (x: BigNumberish, u = 18) => fromWei(x, u);

export interface LiquidationState {
liquidator: string;
liquidatedCollateral: number;
lockedCollateral: number;
liquidatedCollateral: string;
lockedCollateral: string;
liquidationTime: number;
liquidationTimeRemaining: number;
liquidationId: number;
tokensOutstanding: number;
tokensOutstanding: string;
state: number;
}

Expand All @@ -30,10 +30,10 @@ function usePosition() {
const { empState } = EmpState.useContainer();
const { liquidationLiveness } = empState;

const [collateral, setCollateral] = useState<number | null>(null);
const [tokens, setTokens] = useState<number | null>(null);
const [collateral, setCollateral] = useState<string | null>(null);
const [tokens, setTokens] = useState<string | null>(null);
const [cRatio, setCRatio] = useState<number | null>(null);
const [withdrawAmt, setWithdrawAmt] = useState<number | null>(null);
const [withdrawAmt, setWithdrawAmt] = useState<string | null>(null);
const [withdrawPassTime, setWithdrawPassTime] = useState<number | null>(null);
const [pendingWithdraw, setPendingWithdraw] = useState<string | null>(null);
const [pendingTransfer, setPendingTransfer] = useState<string | null>(null);
Expand Down Expand Up @@ -65,15 +65,15 @@ function usePosition() {
position.withdrawalRequestPassTimestamp;
const xferTime: BigNumber = position.transferPositionRequestPassTimestamp;

const collateral: number = weiToNum(collRaw, collDec);
const tokens: number = weiToNum(tokensOutstanding, tokenDec);
const collateral: string = weiToNum(collRaw, collDec);
const tokens: string = weiToNum(tokensOutstanding, tokenDec);
const cRatio =
collateral !== null && tokens !== null
? tokens > 0
? collateral / tokens
? Number(tokens) > 0
? Number(collateral) / Number(tokens)
: 0
: null;
const withdrawAmt: number = weiToNum(withdrawReqAmt, collDec);
const withdrawAmt: string = weiToNum(withdrawReqAmt, collDec);
const withdrawPassTime: number = withdrawReqPassTime.toNumber();
const pendingWithdraw: string =
withdrawReqPassTime.toString() !== "0" ? "Yes" : "No";
Expand Down
10 changes: 6 additions & 4 deletions features/manage-position/Create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ const Create = () => {
const { symbol: tokenSymbol, decimals: tokenDec } = Token.useContainer();
const { gcr } = Totals.useContainer();
const {
collateral: posCollateral,
tokens: posTokens,
collateral: posCollateralString,
tokens: posTokensString,
pendingWithdraw,
} = Position.useContainer();
const { latestPrice } = PriceFeed.useContainer();
Expand All @@ -79,8 +79,8 @@ const Create = () => {
balance !== null &&
collAllowance !== null &&
emp !== null &&
posTokens !== null &&
posCollateral !== null &&
posTokensString !== null &&
posCollateralString !== null &&
minSponsorTokens !== null &&
tokenDec !== null &&
latestPrice !== null &&
Expand All @@ -99,6 +99,8 @@ const Create = () => {
const hasPendingWithdraw = pendingWithdraw === "Yes";
const priceIdentifierUtf8 = hexToUtf8(priceIdentifier);
const prettyLatestPrice = Number(latestPrice).toFixed(4);
const posTokens = Number(posTokensString);
const posCollateral = Number(posCollateralString);

// CR of new tokens to create. This must be > GCR according to https://github.com/UMAprotocol/protocol/blob/837869b97edef108fdf68038f54f540ca95cfb44/core/contracts/financial-templates/expiring-multiparty/PricelessPositionManager.sol#L409
const transactionCR =
Expand Down
12 changes: 7 additions & 5 deletions features/manage-position/Deposit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ const Deposit = () => {
setMaxAllowance,
} = Collateral.useContainer();
const {
tokens: posTokens,
collateral: posColl,
tokens: posTokensString,
collateral: posCollString,
pendingWithdraw,
} = Position.useContainer();
const { latestPrice } = PriceFeed.useContainer();
Expand All @@ -51,8 +51,8 @@ const Deposit = () => {
const [error, setError] = useState<Error | null>(null);

if (
posColl !== null &&
posTokens !== null &&
posCollString !== null &&
posTokensString !== null &&
pendingWithdraw !== null &&
collAllowance !== null &&
collBalance !== null &&
Expand All @@ -63,12 +63,14 @@ const Deposit = () => {
priceIdentifier !== null &&
tokenSymbol !== null &&
collSymbol !== null &&
posColl !== 0 // If position has no collateral, then don't render deposit component.
posCollString !== "0" // If position has no collateral, then don't render deposit component.
) {
const collateralToDeposit = Number(collateral) || 0;
const priceIdentifierUtf8 = hexToUtf8(priceIdentifier);
const hasPendingWithdraw = pendingWithdraw === "Yes";
const collReqFromWei = parseFloat(fromWei(collReq, collDec));
const posTokens = Number(posTokensString);
const posColl = Number(posCollString);
const resultantCollateral = posColl + collateralToDeposit;
const resultantCR = posTokens > 0 ? resultantCollateral / posTokens : 0;
const pricedResultantCR =
Expand Down
26 changes: 14 additions & 12 deletions features/manage-position/Redeem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,16 @@ const MaxLink = styled.div`
text-decoration-line: underline;
`;

const {
formatUnits: fromWei,
parseBytes32String: hexToUtf8,
parseUnits: toWei,
} = utils;
const { formatUnits: fromWei, parseUnits: toWei } = utils;

const Redeem = () => {
const { contract: emp } = EmpContract.useContainer();
const { empState } = EmpState.useContainer();
const { minSponsorTokens } = empState;
const { symbol: collSymbol } = Collateral.useContainer();
const {
tokens: posTokens,
collateral: posColl,
tokens: posTokensString,
collateral: posCollString,
pendingWithdraw,
} = Position.useContainer();
const {
Expand All @@ -58,18 +54,20 @@ const Redeem = () => {
const [error, setError] = useState<Error | null>(null);

if (
posTokens !== null &&
posColl !== null &&
posTokensString !== null &&
posCollString !== null &&
minSponsorTokens !== null &&
tokenBalance !== null &&
tokenDec !== null &&
tokenSymbol !== null &&
tokenAllowance !== null &&
emp !== null &&
pendingWithdraw !== null &&
posColl !== 0 // If position has no collateral, then don't render redeem component.
posCollString !== "0" // If position has no collateral, then don't render redeem component.
) {
const hasPendingWithdraw = pendingWithdraw === "Yes";
const posTokens = Number(posTokensString);
const posColl = Number(posCollString);
const tokensToRedeem =
(Number(tokens) || 0) > posTokens ? posTokens : Number(tokens) || 0;
const minSponsorTokensFromWei = parseFloat(
Expand Down Expand Up @@ -105,7 +103,7 @@ const Redeem = () => {
setSuccess(null);
setError(null);
try {
const tokensToRedeemWei = toWei(tokensToRedeem.toString());
const tokensToRedeemWei = toWei(tokens);
const tx = await emp.redeem([tokensToRedeemWei]);
setHash(tx.hash as string);
await tx.wait();
Expand All @@ -120,7 +118,11 @@ const Redeem = () => {
};

const setTokensToRedeemToMax = () => {
setTokens(tokenBalance.toString());
if (tokenBalance > posTokens) {
setTokens(posTokensString);
} else {
setTokens(tokenBalance.toString());
}
};

if (hasPendingWithdraw) {
Expand Down
12 changes: 8 additions & 4 deletions features/manage-position/SettleExpired.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ const SettleExpired = () => {
isExpired,
} = empState;
const { symbol: collSymbol } = Collateral.useContainer();
const { tokens: posTokens, collateral: posColl } = Position.useContainer();
const {
tokens: posTokensString,
collateral: posCollString,
} = Position.useContainer();
const {
symbol: tokenSymbol,
allowance: tokenAllowance,
Expand All @@ -58,8 +61,8 @@ const SettleExpired = () => {
const [error, setError] = useState<Error | null>(null);

if (
posTokens !== null &&
posColl !== null &&
posTokensString !== null &&
posCollString !== null &&
tokenBalance !== null &&
tokenDec !== null &&
tokenSymbol !== null &&
Expand All @@ -76,7 +79,8 @@ const SettleExpired = () => {
const expiryDate = new Date(
Number(expirationTimestamp) * 1000
).toLocaleString("en-GB", { timeZone: "UTC" });

const posTokens = Number(posTokensString);
const posColl = Number(posCollString);
const needsToRequestSettlementPrice = contractState === CONTRACT_STATE.OPEN;

// Error conditions for calling settle expired:
Expand Down
17 changes: 10 additions & 7 deletions features/manage-position/Withdraw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ const Deposit = () => {
const { symbol: tokenSymbol } = Token.useContainer();
const { symbol: collSymbol, decimals: collDec } = Collateral.useContainer();
const {
tokens: posTokens,
collateral: posColl,
withdrawAmt,
tokens: posTokensString,
collateral: posCollString,
withdrawAmt: withdrawAmtString,
withdrawPassTime,
pendingWithdraw,
} = Position.useContainer();
Expand All @@ -74,24 +74,27 @@ const Deposit = () => {
emp !== null &&
collDec !== null &&
collReq !== null &&
posColl !== null &&
posTokens !== null &&
posCollString !== null &&
posTokensString !== null &&
gcr !== null &&
withdrawPassTime !== null &&
withdrawalLiveness !== null &&
withdrawAmt !== null &&
withdrawAmtString !== null &&
pendingWithdraw !== null &&
currentTime !== null &&
latestPrice !== null &&
priceIdentifier !== null &&
collSymbol !== null &&
tokenSymbol !== null &&
posColl !== 0 // If position has no collateral, then don't render withdraw component.
posCollString !== "0" // If position has no collateral, then don't render withdraw component.
) {
const collateralToWithdraw = Number(collateral) || 0;
const collReqFromWei = parseFloat(fromWei(collReq, collDec));
const priceIdentifierUtf8 = hexToUtf8(priceIdentifier);
const prettyLatestPrice = Number(latestPrice).toFixed(4);
const posTokens = Number(posTokensString);
const posColl = Number(posCollString);
const withdrawAmt = Number(withdrawAmtString);

// CR data:
const resultantCollateral = posColl - collateralToWithdraw;
Expand Down
5 changes: 3 additions & 2 deletions features/manage-position/YourLiquidations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ const YourLiquidations = () => {
const liquidationPretty: any[] = [];
liquidations.map((liq: LiquidationState) => {
let maxDisputablePrice =
liq.tokensOutstanding > 0 && collReqFromWei > 0
? liq.liquidatedCollateral / (liq.tokensOutstanding * collReqFromWei)
Number(liq.tokensOutstanding) > 0 && collReqFromWei > 0
? Number(liq.liquidatedCollateral) /
(Number(liq.tokensOutstanding) * collReqFromWei)
: 0;
if (invertedDisputablePrice) {
maxDisputablePrice = 1 / maxDisputablePrice;
Expand Down
15 changes: 9 additions & 6 deletions features/manage-position/YourPosition.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ const Link = styled.a`
const YourPosition = () => {
const { gcr } = Totals.useContainer();
const {
tokens,
collateral,
tokens: tokenString,
collateral: collString,
cRatio,
withdrawAmt,
withdrawAmt: withdrawAmtString,
pendingWithdraw,
pendingTransfer,
} = Position.useContainer();
Expand All @@ -52,8 +52,8 @@ const YourPosition = () => {
const defaultMissingDataDisplay = "N/A";

if (
tokens !== null &&
collateral !== null &&
tokenString !== null &&
collString !== null &&
cRatio !== null &&
gcr !== null &&
collSymbol !== null &&
Expand All @@ -62,14 +62,17 @@ const YourPosition = () => {
latestPrice !== null &&
collReq !== null &&
priceIdentifier !== null &&
withdrawAmt !== null &&
withdrawAmtString !== null &&
pendingWithdraw !== null &&
pendingTransfer !== null
) {
const pricedCR =
latestPrice !== 0 ? (cRatio / latestPrice).toFixed(4) : "0";
const pricedGCR = latestPrice !== 0 ? (gcr / latestPrice).toFixed(4) : "0";
const collReqFromWei = parseFloat(fromWei(collReq, collDec));
const tokens = Number(tokenString);
const collateral = Number(collString);
const withdrawAmt = Number(withdrawAmtString);
const liquidationPrice = getLiquidationPrice(
collateral,
tokens,
Expand Down
2 changes: 1 addition & 1 deletion features/weth/Weth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ const Weth = () => {
setSuccess(null);
setError(null);
try {
const amountWei = toWei(wethAmountToUnwrap.toString());
const amountWei = toWei(wethAmount);
let tx = await weth.withdraw(amountWei);
setHash(tx.hash as string);
await tx.wait();
Expand Down

1 comment on commit f2922a2

@vercel
Copy link

@vercel vercel bot commented on f2922a2 Aug 11, 2020

Choose a reason for hiding this comment

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

Please sign in to comment.