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

[PDB-01M] Arbitrary External Contract Calls #894

Merged
merged 17 commits into from
Jan 23, 2024
Merged

Conversation

zajck
Copy link
Member

@zajck zajck commented Jan 19, 2024

Fix #861

This change moves a part of price discovery logic outside the diamond, effectively creating another boson protocol client.

Since this client does not own any assets, they cannot be stolen from it. The user will always approve only Boson Protocol or external price discovery contract to transfer the vouchers and/or erc20 tokens.

Users can still make arbitrary calls through this client, but since it's outside the boson protocol address, any calls that the client makes will not be directly associated with the boson protocol.

@zajck zajck added bug Something isn't working v2.4.0 labels Jan 19, 2024
@zajck zajck self-assigned this Jan 19, 2024
@coveralls
Copy link

coveralls commented Jan 19, 2024

Coverage Status

coverage: 98.922% (+0.08%) from 98.839%
when pulling ffb3698 on audit_v2_4_0_pdb_01m
into 4d6b75c on main.

import { Address } from "@openzeppelin/contracts/utils/Address.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

// import { BosonPriceDiscovery } from "./../clients/priceDiscovery/BosonPriceDiscovery.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

remove unused code

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

/**
* @title BosonPriceDiscovery
*
* @dev Boson Price Discovery is a external contract that is used to determine the price of an exchange.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @dev Boson Price Discovery is a external contract that is used to determine the price of an exchange.
* @dev Boson Price Discovery is an external contract that is used to determine the price of an exchange.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.


/**
* @notice
* For offers with native exchange token, it is expected the the price discovery contracts will
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* For offers with native exchange token, it is expected the the price discovery contracts will
* For offers with native exchange token, it is expected that the price discovery contracts will

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

/**
* @notice
* For offers with native exchange token, it is expected the the price discovery contracts will
* operate with wrapped native token. Set the address of the wrapped native token in the constructor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* operate with wrapped native token. Set the address of the wrapped native token in the constructor.
* operate with wrapped native token. Sets the address of the wrapped native token in the constructor.

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept that as it was, since it's meant as an instruction for the deployer.

levalleux-ludo
levalleux-ludo previously approved these changes Jan 22, 2024
contracts/protocol/facets/ExchangeHandlerFacet.sol Outdated Show resolved Hide resolved
@zajck zajck merged commit 4ef47b6 into main Jan 23, 2024
12 checks passed
@zajck zajck deleted the audit_v2_4_0_pdb_01m branch January 23, 2024 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PDB-01M] Arbitrary External Contract Calls
4 participants