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

Add C-chain wallet to the primary network #1850

Merged
merged 23 commits into from
Aug 19, 2023
Merged

Add C-chain wallet to the primary network #1850

merged 23 commits into from
Aug 19, 2023

Conversation

StephenButtolph
Copy link
Contributor

@StephenButtolph StephenButtolph commented Aug 14, 2023

Why this should be merged

This significantly reduces the friction of issuing import/export txs for the C-chain on the primary network when using the avalanchego wallet.

How this works

  • Adds a C-chain wallet to the Primary network wallet that can issue import + export txs.
  • Adds an ethereum compatible keychain to the config passed into MakeWallet
  • Adds examples running imports + exports to/from the C-chain.

How this was tested

Ran the examples locally.

go.mod Outdated Show resolved Hide resolved
Comment on lines +175 to +177
func publicKeyToEthAddress(pk *secp256k1.PublicKey) common.Address {
return crypto.PubkeyToAddress(*(pk.ToECDSA()))
}
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 is also implemented in coreth - but would cause a circular import. I'd be willing to expose this here if we wanted coreth to just depend on this and we remove the duplicated code

avaxAddrToKeyIndex map[ids.ShortID]int
ethAddrToKeyIndex map[common.Address]int

// These can be used to iterate over. However, they should not be modified
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) Maybe that suggests making them private and returning a copy? Or do performance concerns trump enforcing immutability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There shouldn't be anything performance critical using the keychain. (as it is only used to sign transactions)

wallet/chain/c/backend.go Outdated Show resolved Hide resolved
Base automatically changed from cleanup-wallet-initialization to dev August 15, 2023 21:57
Copy link
Contributor

@marun marun left a comment

Choose a reason for hiding this comment

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

A couple of nits but mainly questions to educate myself.

wallet/chain/c/backend.go Show resolved Hide resolved
wallet/chain/c/builder.go Outdated Show resolved Hide resolved
wallet/chain/c/builder.go Outdated Show resolved Hide resolved
wallet/chain/c/builder.go Outdated Show resolved Hide resolved
wallet/chain/c/builder.go Outdated Show resolved Hide resolved
wallet/chain/c/builder.go Show resolved Hide resolved
wallet/chain/c/builder_with_options.go Show resolved Hide resolved
wallet/chain/c/signer.go Show resolved Hide resolved
wallet/chain/c/wallet.go Outdated Show resolved Hide resolved
wallet/chain/c/wallet.go Outdated Show resolved Hide resolved
@StephenButtolph StephenButtolph added this to the v1.10.9 milestone Aug 17, 2023
@StephenButtolph StephenButtolph marked this pull request as ready for review August 17, 2023 17:47
@StephenButtolph
Copy link
Contributor Author

StephenButtolph commented Aug 17, 2023

I'm going to start working on a followup PR to abstract the baseFee from the wallet and allow issuance of ethereum transactions (not import/export txs)

EDIT: This wallet is only going to support import/export txs for now. We may add it later... but because of ethereum's execution flow... it isn't going to be very easy to track balances for non-avax txs.

@StephenButtolph StephenButtolph added the sdk This involves SDK tooling or frameworks label Aug 17, 2023
@marun
Copy link
Contributor

marun commented Aug 18, 2023

Automatic baseFee determination lgtm.

Copy link
Contributor

@dhrubabasu dhrubabasu left a comment

Choose a reason for hiding this comment

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

one small typo but LGTM

@StephenButtolph StephenButtolph merged commit ea4475b into dev Aug 19, 2023
16 checks passed
@StephenButtolph StephenButtolph deleted the c-chain-wallet branch August 19, 2023 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sdk This involves SDK tooling or frameworks
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants