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

Automated Gas Payments #312

Merged
merged 49 commits into from
Jul 2, 2024
Merged

Automated Gas Payments #312

merged 49 commits into from
Jul 2, 2024

Conversation

peculiarity
Copy link
Contributor

No description provided.

src/CentrifugeRouter.sol Outdated Show resolved Hide resolved
src/CentrifugeGasService.sol Outdated Show resolved Hide resolved
src/CentrifugeGasService.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/gateway/routers/Aggregator.sol Outdated Show resolved Hide resolved
src/gateway/routers/Aggregator.sol Outdated Show resolved Hide resolved
src/gateway/routers/axelar/Router.sol Outdated Show resolved Hide resolved
src/CentrifugeRouter.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/gateway/routers/axelar/Router.sol Outdated Show resolved Hide resolved
src/gateway/routers/Aggregator.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
script/Deployer.sol Outdated Show resolved Hide resolved
src/CentrifugeRouter.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/GasService.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/CentrifugeRouter.sol Outdated Show resolved Hide resolved
@peculiarity peculiarity force-pushed the gg/feat/automate-gas-payments branch from 56c7469 to ae78307 Compare June 10, 2024 15:18
/// @inheritdoc ICentrifugeRouter
function requestRedeem(address vault, uint256 amount, address controller, address owner) external protected {
function requestRedeem(address vault, uint256 amount, address controller, address owner)
Copy link
Contributor

Choose a reason for hiding this comment

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

why aren't we topping up the gateway here. The user has to pay for gas here as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know. I haven't put all the methods. Thought for the MVP we wanted to have a single method

src/InvestmentManager.sol Outdated Show resolved Hide resolved
src/PoolManager.sol Outdated Show resolved Hide resolved
src/PoolManager.sol Outdated Show resolved Hide resolved
src/PoolManager.sol Outdated Show resolved Hide resolved
else revert("Gateway/file-unrecognized-param");
emit File(what, data1, data2);
}

// --- Incoming ---
/// @inheritdoc IGateway
Copy link
Contributor

Choose a reason for hiding this comment

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

handle has to be auth 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we agreed that it's fine right now. Only routers can call handle. Correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree this is correct as is.

function topUp() external payable {
require(RootLike(root).endorsed(msg.sender), "Gateway/only-endorsed-can-topup");
require(msg.value > 0, "Gateway/cannot-topup-with-nothing");
quota = msg.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need the extra quota variable, would just use fuel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some renaming. quota is updated only by CentrifugeRouter. If there is no quote assigned of the transaction, then we use the gateway's balance .

src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/gateway/routers/Aggregator.sol Outdated Show resolved Hide resolved
src/PoolManager.sol Outdated Show resolved Hide resolved
src/gateway/Gateway.sol Outdated Show resolved Hide resolved
src/libraries/MessagesLib.sol Outdated Show resolved Hide resolved
else revert("Gateway/file-unrecognized-param");
emit File(what, data1, data2);
}

// --- Incoming ---
/// @inheritdoc IGateway
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree this is correct as is.

@ilinzweilin ilinzweilin self-requested a review June 19, 2024 19:43
@@ -58,8 +57,13 @@ contract Deployer is Script {
investmentManager = new InvestmentManager(address(root), address(escrow));
poolManager = new PoolManager(address(escrow), vaultFactory, restrictionManagerFactory, trancheTokenFactory);

// TODO THESE VALUES NEEDS TO BE CHECKED
gasService = new GasService(20000000000000000, 20000000000000000, 2500000000000000000, 178947400000000);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move these to env vars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that once we come up wuth the actual values, we can put the into env vars :D

peculiarity and others added 10 commits June 20, 2024 00:54
* chore: Migrate all Aggregator tests to be applicable for gateway

* Format

---------

Co-authored-by: Jeroen Offerijns <[email protected]>
* Sepolia deployment

* Format

* Undo router values

* cleanup
#331)

* chore: Move responsibility fo handling an exception from the caller to the callee ( method itself )

* chore: Fix formatting
…#334)

* fix: A way to call topUp with different owner than the CR  and a dummy vault causing an issue in the flow execution.

* fix: The check for vault should happen even when the owner is the CR address
…nsaction not triggered by CentrifugeRouter (#335)

* fea:(Gateway) Allow partial payments from the gateway balance for transaction not triggered by CentrifugeRouter

* chore: A forgotten file to be renamed
Copy link

github-actions bot commented Jul 1, 2024

Coverage after merging gg/feat/automate-gas-payments into main will be

87.26%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   Auth.sol100%100%100%100%
   CentrifugeRouter.sol60.16%50%70%62.32%100–101, 101, 101–102, 104, 106–108, 115, 130, 151, 151, 161, 165, 175, 180, 182–183, 183–184, 186–187, 187, 191–192, 192, 194, 194, 200, 205, 207, 224, 33–35, 37–38, 42, 49, 49, 72, 85
   ERC7540Vault.sol82.42%70.83%93.02%80.61%107, 130, 139, 181–183, 208–210, 236–238, 250, 70–76, 78–81, 83–84
   Escrow.sol100%100%100%100%
   InvestmentManager.sol90.65%88.04%86.36%92.97%134, 253, 262, 306, 335, 357, 389, 393–394, 516, 559–562, 571–574, 574, 574, 60–61, 63–64
   PoolManager.sol91.98%93.97%75%93.33%212, 214, 214, 214–215, 217, 480, 480, 480–481, 481, 481–482, 70–73, 75–76
   Root.sol92.11%88.89%92.86%93.18%124, 129, 149–150, 38
src/admin
   Guardian.sol66.67%50%77.78%63.64%23–25, 29, 29, 63
src/factories
   ERC7540VaultFactory.sol100%100%100%100%
   RestrictionManagerFactory.sol66.67%100%50%70%23, 25–26
   TrancheTokenFactory.sol100%100%100%100%
   TransferProxyFactory.sol100%100%100%100%
src/gateway
   GasService.sol30.56%25%33.33%31.82%25–29, 31–32, 37, 37, 37–38, 38, 38–40, 45, 45, 45–47, 52
   Gateway.sol89.11%82.56%82.35%93.51%161–162, 166, 205, 208–209, 212, 220, 256, 262, 279, 288, 343, 343, 343–344, 346, 62–65, 67–68, 83, 85
src/gateway/adapters/axelar
   Adapter.sol50%80%33.33%37.50%49–51, 53–54, 59, 59, 59–61, 91, 99
   Forwarder.sol80%100%75%75%38, 40–41
src/token
   ERC20.sol97.12%94.12%100%98.31%161, 60–61
   RestrictionManager.sol94.83%100%66.67%100%
   Tranche.sol77.14%62.50%72.73%87.50%55, 64, 73, 83, 93

@peculiarity peculiarity merged commit b87388f into main Jul 2, 2024
4 checks passed
@peculiarity peculiarity deleted the gg/feat/automate-gas-payments branch July 2, 2024 09:07
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

Successfully merging this pull request may close these issues.

None yet

3 participants