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

[WIP] add ed25519 sig verification precompile #16453

Closed
wants to merge 1 commit into from

Conversation

oberstet
Copy link

@oberstet oberstet commented Apr 6, 2018

this is a first take at https://github.com/ethereum/EIPs/blob/master/EIPS/eip-665.md - compiles and runs, but not yet tested. I am putting this up to see the CI results, have a reference in the dev meeting, and gather feedback. also missing, bits in core/vm/contracts_test.go and eth/tracers/tracer.go:

oberstet@thinkpad-t430s:~/scm/oberstet/go-ethereum$ find . -type f -exec grep -Hi "PrecompiledContractsByzantium" {} \;
./core/vm/contracts_test.go:	p := PrecompiledContractsByzantium[common.HexToAddress(addr)]
./core/vm/contracts_test.go:	p := PrecompiledContractsByzantium[common.HexToAddress(addr)]
./core/vm/evm.go:			precompiles = PrecompiledContractsByzantium
./core/vm/evm.go:			precompiles = PrecompiledContractsByzantium
./core/vm/contracts.go:// PrecompiledContractsByzantium contains the default set of pre-compiled Ethereum
./core/vm/contracts.go:var PrecompiledContractsByzantium = map[common.Address]PrecompiledContract{
./eth/tracers/tracer.go:		_, ok := vm.PrecompiledContractsByzantium[common.BytesToAddress(popSlice(ctx))]
Übereinstimmungen in Binärdatei ./build/bin/geth
oberstet@thinkpad-t430s:~/scm/oberstet/go-ethereum$ find . -type f -exec grep -Hi "PrecompiledContractsConstantinople" {} \;
./core/vm/evm.go:			precompiles = PrecompiledContractsConstantinople
./core/vm/evm.go:			precompiles = PrecompiledContractsConstantinople
./core/vm/contracts.go:// PrecompiledContractsConstantinople contains the default set of pre-compiled Ethereum
./core/vm/contracts.go:var PrecompiledContractsConstantinople = map[common.Address]PrecompiledContract{
Übereinstimmungen in Binärdatei ./build/bin/geth
oberstet@thinkpad-t430s:~/scm/oberstet/go-ethereum$ 

@GitCop
Copy link

GitCop commented Apr 6, 2018

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 3272ba2d30e41be264cd2d3ff0b9bec762c3b98d
  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@oberstet
Copy link
Author

oberstet commented Apr 6, 2018

Q1: How to modify ./eth/tracers/tracer.go for both PrecompiledContractsByzantium and PrecompiledContractsConstantinople:

	tracer.vm.PushGlobalGoFunction("isPrecompiled", func(ctx *duktape.Context) int {
		_, ok := vm.PrecompiledContractsByzantium[common.BytesToAddress(popSlice(ctx))]
		ctx.PushBoolean(ok)
		return 1
	})

Q2: How to modify ./core/vm/contracts_test.go

func testPrecompiled(addr string, test precompiledTest, t *testing.T) {
	p := PrecompiledContractsByzantium[common.HexToAddress(addr)]
	in := common.Hex2Bytes(test.input)
	contract := NewContract(AccountRef(common.HexToAddress("1337")),
		nil, new(big.Int), p.RequiredGas(in))
	t.Run(fmt.Sprintf("%s-Gas=%d", test.name, contract.Gas), func(t *testing.T) {
		if res, err := RunPrecompiledContract(p, in, contract); err != nil {
			t.Error(err)
		} else if common.Bytes2Hex(res) != test.expected {
			t.Errorf("Expected %v, got %v", test.expected, common.Bytes2Hex(res))
		}
	})
}

@oberstet
Copy link
Author

oberstet commented Apr 6, 2018

I don't get the failing CI step: https://travis-ci.org/ethereum/go-ethereum/jobs/363155865#L729 - this is stalled in generating coverage reports .. mmh. not sure what to do ..

@holiman
Copy link
Contributor

holiman commented Apr 9, 2018

Don't worry about the travis, we're having some problems with long-running builds that time out on travis from time to time.

@oberstet
Copy link
Author

oberstet commented Apr 9, 2018

@holiman thanks for clarifying! could you maybe retrigger the failing ones, so the PR becomes "green" (=> more likely being looked at)?

@holiman
Copy link
Contributor

holiman commented Jan 17, 2019

As this has not been accepted as a HF, we're not going to merge this.
Since we're not going to merge this (anytime soon), I'll close this PR. If this ever is accepted, please reopen this.

@holiman holiman closed this Jan 17, 2019
@SCBuergel
Copy link

@holiman - is there any TLDR on why this EIP became stagnant and didn't get included despite looking pretty ready? We (HOPR) would currently significantly benefit from having ed25519 onchain. It would be great to have transparency on if this ever happens or what other paths might have been discussed.

@oberstet
Copy link
Author

fwiw, the upstream EIP665 never got accepted as eWASM was suggested as the way forward instead of more and more precompiles ... well, yes, on the one hand. on the other, I still can't deploy eWASM contracts on Ethereum in 2023?

@holiman
Copy link
Contributor

holiman commented Jun 13, 2023

is there any TLDR on why this EIP became stagnant and didn't get included despite looking pretty ready?

Sorry, I don't know the full history, but it's from 2018 and as far as I know there's noone championing this eip for inclusion. However, the discussion about whether to apply EIPs changes to the Ethereum protocol does not happen at this repo, but typically at FEM

@rdubois-crypto
Copy link

Hello. At Ledger we would find some interest in the EIP: enabling WebAuthn with Ed25519 with Account Abstraction. The given address was obsolete, and didn't found any discussion in FEM, so one has been created here:
https://ethereum-magicians.org/t/eip-665-ed25519-signature-verification/15535

First we planned to submit a new EIP targeting only the ECC computation, to allow one to choose any Hash function, (for instance a zk friendly one), then realized 665 existed. @oberstet , are you open to a discussion ? (provided given email address seems to be expired).

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.

6 participants