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

EIP-3074 #2636

Closed
wants to merge 6 commits into from
Closed

EIP-3074 #2636

wants to merge 6 commits into from

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Aug 14, 2021

Signed-off-by: Antoine Toulme antoine@lunar-ocean.com

PR description

This adds the auth and authcall opcodes of EIP-3074 to Besu, as well as the Puxi testnet definition.

The PR is not ready for merge. It needs thorough review.

See #2637

Changelog

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
@atoulme atoulme changed the title Add auth opcode EIP-3074 Aug 15, 2021
Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
…yet, since authcall allows to send from completely new accounts

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
Copy link

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Overall looking pretty good :)

}

@Override
protected Wei apparentValue(final MessageFrame frame) {

Choose a reason for hiding this comment

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

What does "apparent" refer to here?


@Override
protected UInt256 inputDataOffset(final MessageFrame frame) {
return frame.getStackItem(3);

Choose a reason for hiding this comment

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

Looks like valueExt is missing here, it should be the top - 3 stack element.

Optional.of(cost(frame)), Optional.of(ExceptionalHaltReason.INSUFFICIENT_GAS));
}
}
return super.execute(frame, evm);

Choose a reason for hiding this comment

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

Can't seem to find the logic to ensure that if the gas operand is 0, all but one 64th of the remaining gas is passed to the subcall.

}

@Override
protected OperationResult executeFixedCostOperation(final MessageFrame frame, final EVM evm) {

Choose a reason for hiding this comment

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

Not sure if someone up the call tree authorized is cleared, but in the case of a ecrecover failure it should clear the current authorized address and push 0 on the stack.

@lightclient
Copy link

lightclient commented Aug 26, 2021

FYI, we made a small update to EIP-3074 to include chainId in the authorization message, padded to 32 bytes. You can see the diff of that change to the EIP here: ethereum/EIPs#3769

I think that change should pretty much resolve all outstanding issues we have with 3074, so I think it is the now the most stable we can expect it to be.

@atoulme
Copy link
Contributor Author

atoulme commented Jan 3, 2022

Going to close as this PR is now old, and didn't follow the latest changes of the EIP.

@atoulme atoulme closed this Jan 3, 2022
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

2 participants