Giveth / vaultcontroller

Vault Architecture
GNU General Public License v3.0
0 stars 3 forks source link

Vault cannot be cancelled if balance is zero #7

Closed adria0 closed 7 years ago

adria0 commented 7 years ago

in cancelVault, vault is not cancelled if balance is zero

        uint vaultBalance = primaryVault.getBalance();

        if ((vaultBalance > 0) || (!canceled)) {
            primaryVault.authorizePayment(
              "CANCEL CHILD VAULT",
              bytes32(msg.sender),
              address(parentVault),
              vaultBalance,
              0
            );
            canceled = true;
            highestAcceptableBalance = 0;
            lowestAcceptableBalance = 0;
            owner = parentVaultController;
            VaultCanceled(msg.sender);
        }
    }

could be changed to

        uint vaultBalance = primaryVault.getBalance();

        if (!canceled) {
            if (vaultBalance > 0) { 
                primaryVault.authorizePayment(
                  "CANCEL CHILD VAULT",
                  bytes32(msg.sender),
                  address(parentVault),
                  vaultBalance,
                  0
                );
            }
            canceled = true;
            ...
        }
    }

and since we don't have control about wrapped token, why not to protect against unknown recursive attacks:

        uint vaultBalance = primaryVault.getBalance();

        if (!canceled) {
            canceled = true;
            if (vaultBalance > 0) { 
                primaryVault.authorizePayment(
                  "CANCEL CHILD VAULT",
                  bytes32(msg.sender),
                  address(parentVault),
                  vaultBalance,
                  0
                );
            } 
            ...
        }
    }
jbaylina commented 7 years ago

Fixed.

Please rereview it!.