Skip to content

Conversation

@adhusson
Copy link
Contributor

@adhusson adhusson commented Nov 9, 2024

Lock down bundle content and ordering. See issue #61.

multicall has a 2nd argument: bytes32[] callbackBundlesHashes. The argument of the nth call to multicallFromBundler is hashed and checked against the nth callbackBundlesHashes.

edit: compared to #165 / #167:

  • with this PR, multiple reenters within the same Call are possible
  • with this PR, the order of bundles can be checked but not the fact that a specific adapter reenter with a specific bundle.

@adhusson adhusson changed the title Callback bundles cannot be altered during bundle execution experimental: lockdown bundles Nov 9, 2024
@adhusson adhusson marked this pull request as ready for review November 9, 2024 13:52
@adhusson adhusson changed the title experimental: lockdown bundles experimental: lockdown bundles before execution Nov 9, 2024
src/Hub.sol Outdated
function multicallFromBundler(Call[] calldata bundle) external payable {
require(msg.sender == currentBundler(), ErrorsLib.UnauthorizedSender(msg.sender));
_multicall(calls);
require(useBundleHash() == keccak256(callDataArgs()), ErrorsLib.InvalidBundle());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only checks that the stored bundle hash matches msg.data[4:].

It is technically possible to send extra calldata after the bundle argument such that bundle does not hash correctly and msg.data does hash correctly, but that would mean a hash collision was found.

Base automatically changed from feat/star-pattern to main November 16, 2024 23:19
@adhusson
Copy link
Contributor Author

Closing because the main usecase is a module that interacts with changing / upgradeable contracts. In this scenario the module can do the bundle integrity check itself.

@adhusson adhusson changed the title experimental: lockdown bundles before execution feat: lockdown bundles before execution Dec 8, 2024
@adhusson adhusson changed the base branch from main to dev December 8, 2024 19:37
@MathisGD
Copy link
Contributor

MathisGD commented Dec 9, 2024

Replaced by #165 or #170 that are simpler and simpler to use

@MathisGD MathisGD closed this Dec 9, 2024
@MathisGD MathisGD deleted the feat/callback-hashes-in-transient-storage branch December 9, 2024 08:33
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.

3 participants