fix: multisig support call invoke contract

[go-lifei]

Related Issues

multi wallet call contract need encode evm params

Proposed Changes

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

• Commits have a clear commit message.
• PR title is in the form of of <PR type>: <area>: <change being made>
  • example: fix: mempool: Introduce a cache for valid signatures
  • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
  • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
• If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
• New features have usage guidelines and / or documentation updates in
  • Lotus Documentation
  • Discussion Tutorials
• Tests exist for new functionality or change in behavior
• CI is green

[rjan90]

Thanks for the PR @go-lifei ! Could you base your PR on the master branch, instead of the release/v1.26.0 branch. And for fixing a couple of the failing checks, you need to run make gen and make docsgen in you /lotus folder.

[Stebalien]

This is a bit too magical for my liking, and it'll break any users who are manually cbor-encoding their multisig parameters.

Maybe we could allow the user to pass the literal string invoke-evm (or evm-call?) instead of the method number?

@rvagg any opinions here?

[rvagg]

yeah, branching just for that case and handling input differently is a bit quirky

what's the workflow here that we're trying to solve for? how are you generating your parameters hex value today that you'd call this with?

[go-lifei]

how are you generating your parameters hex value today that you'd call this with?

i used my local tool or code . lol.

[rvagg]

my local tool or code

right, so is there a difficulty in cbor encoding the resultant bytes yourself before outputting as hex if this is coming from existing tooling?

[go-lifei]

my local tool or code

right, so is there a difficulty in cbor encoding the resultant bytes yourself before outputting as hex if this is coming from existing tooling?

easy for me . but difficulty for any other customer.

[rvagg]

I'm not sure of the best course of action here tbh. I don't think we should break existing workflows that assume it needs to be cbor encoded before passing. Perhaps a new option (--encode-invoke). Or perhaps an entirely new command for this. @Stebalien your judgement is going to be far better than mine on this.

[Stebalien]

I don't think a flag will work because the method number is currently a positional argument. We could add entirely new commands, but I expect we will eventually want to support contract deployment and other types of invocations, I'm concerned we will end up with a bunch of commands (propose, approve, cancel / call, create).

[rvagg]

OK, circling back to this, I think I like the original suggestion of adding a new way of specifying the message, instead of a number, allow for the string "invoke-evm" and if that's supplied instead of a number, then CBOR encode the bytes and use MethodsEVM.InvokeContract as the method.

@go-lifei would you be OK adjusting this PR for that proposal? It'd mean adjusting the logic around the m, err := strconv.ParseUint lines to get the right method int and an additional boolean that tells you whether to encode cbor bytes. It'll also need some additional documentation, perhaps the ArgsUsage could be adjusted for each of them to show the new possibility, like: multisigAddress destinationAddress value <methodId|"invoke-evm" methodParams> (optional)

[rjan90]

Hey! 👋

As part of our cleanup to kick off the year, I'm reviewing all open non-draft pull requests. Could you please do one of the following for your PR?

1. Close it: If it's no longer needed.
2. Mark as Draft: If it needs more work, and add the next steps.
3. Ready for Review: If it's good to go, let me know, and I'll assign a reviewer.

If there's no response in a week, I'll assume it's option 1 and close the PR. If you have any questions, just let me know.

Thanks for your help in keeping things organized, and I appreciate your contributions!

[rjan90]

If there's no response in a week, I'll assume it's option 1 and close the PR.

Closing