ampleforth-contracts
ampleforth-contracts copied to clipboard
Orchestrator V2
Contributors can pick up of the issues listed here (all are good first issues, great to get started):
-
[x] The Orchestrator was created to disregard failed transactions. However it was patched to revert if one of the listed transactions fail (to prevent a gas underprice attack). Update the inline code documentation to reflect the patched change].
-
[x] Replace
externalCall
with.call
-
[ ] The orchestrator imports the entire
UFragmentsPolicy.sol
. The typical way to address this is to just import the relevant interface, instead of importing the full contract. This helps reduce the deployed contract size. -
[ ] Use the revert message to indicate the index of the external call transaction which failed
-
[ ] Use non upgradable version of
Ownable
in The Orchestrator contract. -
[ ] The current Orchestrator contract uses require(msg.sender == tx.origin) to prevent contracts from calling rebase. But tx.origin may be deprecated in the future. Implement one of these alternate mechanisms described here.
@nithinkrishna Can you explain a little more about Replace externalCall with .call
? I see that externalCall function already has a .call function wrapped inside of the inline assembly.
@tedwu13 With solidity 0.4.24 we had to use assembly to make external calls. With later versions you can use address.call(data)
to make an external function call.
The idea is to get rid of the externalCall
method we've defined and just use the native solidity method.
https://docs.soliditylang.org/en/v0.7.0/control-structures.html?highlight=call#external-function-calls
# something like?
Transaction storage t = transactions[i];
(bool result, bytes memory reason) = t.destination.call(t.data);
@nithinkrishna I think tx.origin
is here to stay in order to protect backwards compatibility - see below.
https://github.com/ethereum/solidity/issues/683