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

Fix Go client contract deployment #315

Merged
merged 2 commits into from
May 20, 2024
Merged

Fix Go client contract deployment #315

merged 2 commits into from
May 20, 2024

Conversation

aefhm
Copy link
Contributor

@aefhm aefhm commented May 15, 2024

Description

PackSignedCall is part of the deployment flow and needs to support the case where msg.To is nil.

func PackSignedCall(msg ethereum.CallMsg, cipher Cipher, sign SignerFn, chainID big.Int, leash *evm.Leash) (*ethereum.CallMsg, error) {
if msg.Gas == 0 {
msg.Gas = DefaultGasLimit // Must be non-zero for signed calls.
}
if msg.GasPrice == nil {
msg.GasPrice = big.NewInt(DefaultGasPrice) // Must be non-zero for signed calls.
}
dataPack, err := evm.NewSignedCallDataPack(rsvSigner{sign}, chainID.Uint64(), msg.From[:], msg.To[:], msg.Gas, msg.GasPrice, msg.Value, msg.Data, *leash)

TODO

  • Publish new Go client.
  • Test new Go client.
  • Compare against Python client.

@aefhm aefhm self-assigned this May 15, 2024
@aefhm aefhm requested a review from matevz May 15, 2024 23:24
Copy link

netlify bot commented May 15, 2024

Deploy Preview for oasisprotocol-sapphire-paratime ready!

Name Link
🔨 Latest commit 7ed30b5
🔍 Latest deploy log https://app.netlify.com/sites/oasisprotocol-sapphire-paratime/deploys/66466fd6d22d55000880dbea
😎 Deploy Preview https://deploy-preview-315--oasisprotocol-sapphire-paratime.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aefhm aefhm added p:0 Priority: very high dependencies Pull requests that update a dependency file go Pull requests that update Go code client blocked Pull requests that are blocked by another issue and removed p:0 Priority: very high labels May 15, 2024
Copy link
Member

@matevz matevz left a comment

Choose a reason for hiding this comment

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

The app crashes when you want to deploy a contract with an encrypted transaction right? We're using this go-client library in network-monitoring, but we didn't encounter any crashes, probably because we use the unwrapped version of the eth client for the contract deployment.

Can you also please fix the linting issues while you're at it.

clients/go/compat.go Outdated Show resolved Hide resolved
@aefhm aefhm changed the title Fix Go client deploy Fix Go client contract deployment May 16, 2024
@aefhm
Copy link
Contributor Author

aefhm commented May 16, 2024

The app crashes when you want to deploy a contract with an encrypted transaction right?

I'm using plain. 🤔

probably because we use the unwrapped version of the eth client for the contract deployment.

Which version of the client?

@aefhm
Copy link
Contributor Author

aefhm commented May 20, 2024

I think my QA issues are independent of this client patch.

@aefhm aefhm merged commit b8aa60c into main May 20, 2024
15 checks passed
@aefhm aefhm deleted the xz/fix-go-client-deploy branch May 20, 2024 04:16
@aefhm
Copy link
Contributor Author

aefhm commented May 20, 2024

@kostko new release to clients/go/v0.11.2 please?

@kostko
Copy link
Member

kostko commented May 20, 2024

Done.

@aefhm aefhm removed the blocked Pull requests that are blocked by another issue label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client dependencies Pull requests that update a dependency file go Pull requests that update Go code p:0 Priority: very high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants