Closed alexyoung23j closed 1 year ago
Updated label as this appears to be a blocker, not blocked. @alexyoung23j
Here is the example template we have been using (this is not the template that is blocked, obviously.)
form_fields: [
{
text: {
field_name: 'subtitle',
field_type: 'h2',
field_value: 'Transfer Tokens',
},
},
{
input: {
field_name: 'address',
field_label: "Who's the recipient?",
field_ref: 'address-ref',
formatter: 'address',
},
},
{
input: {
field_name: 'amount',
field_label: 'How much should we transfer to them (decimal 18)?',
field_ref: 'amount-ref',
formatter: 'token',
},
},
],
tx_template: {
method: 'transfer',
args: {
to: '$address-ref',
amount: '$amount-ref',
},
},
};```
Here's the template that is supposed to work but results in this issue:
"form_fields": [
{
"text": {
"field_name": "title",
"field_type": "h1",
"field_value": "Create a New Proposal"
}
},
{
"input": {
"field_name": "name",
"field_label": "Enter proposal title",
"field_placeholder": "example title",
"field_ref": "name-ref",
"formatter": "string"
}
},
{
"divider": {
"field_name": "divider"
}
},
{
"text": {
"field_name": "subtitle",
"field_type": "h2",
"field_value": "Parameter Change: Set Tax"
}
},
{
"input": {
"field_name": "tax man tax man",
"field_label": "Set the tax man",
"field_ref": "tax-ref",
"formatter": "number"
}
}
],
"tx_template": {
"method": "propose",
"args": {
"target": [
"0xProtocolAddress"
],
"calldata": [
"setTax"
],
"values": [
"$tax-ref"
],
"description": "$name-ref"
}
}
}```
At first look here the issue as I understand it is that we need to be able nest a tx formatter within a proposal(or other) tx template where calldata(or signatures, etc) bytes are requested.
An initial thought is that the creator of a template will not typically know all of the functions they may want to nest in the future, so I think its valuable to abstract that out to our own mapping/data. To achieve this we should store(db or FE) a mapping of solidity param types to default input field producers.
(ie uint256 => Numerical input, string/bytes => any string input, address => string input w/ addr validation)
If we do this, we can follow a flow like the following for a proposal builder:
for a governor bravo proposal propose(address[] targets, uint256[] values, bytes[] calldatas, string description)
User may set up a template as follows:
form_fields: [
{
text: {
field_name: 'subtitle',
field_type: 'h2',
field_value: 'Transfer Tokens',
},
},
{
input: {
field_name: 'address',
field_label: "List of target Contract addresses",
field_ref: 'address-ref',
formatter: 'address-list',
},
},
{
input: {
field_name: 'values',
field_label: 'Value in ETH to send to each address',
field_ref: 'value-ref',
formatter: 'value',
},
FunctionBuilderSelection: {
field_name: 'Calldata',
field_label: 'The Calldata to send to each Contract address',
field_ref: 'calldata-ref',
contract_link: 'address-ref'
formatter: 'function-data',
},
},
],
tx_template: {
method: 'propose',
args: {
address[]: '$address-ref',
uint256[]: '$value-ref',
bytes[]: '$calldata-ref',
string: '$description-ref'
},
},
};`
I'm probably doing some things wrong here based on our current implementation but here's the parts I meant to include:
Our FE parses this form as usual. Upon a FunctionBuilderSelector field
web3.abi.encodeParameters()
Encode all parameters including built call data and send tx.
This is just a rough sketch and I know we'll need to think through how to add each item of call data(maybe a + button to add to array up to the length of the address/target array)
"tx_template": { "method": "propose", "args": { "target": [ "0xProtocolAddress" ], "calldata": [ "setTax" ], "values": [ "$tax-ref" ],
In this case and in most governance contracts, 'value' refers to the ETH value to be sent with a tx. So here, we're setting the param for SetTax as value where as we should be encoding the calldata to support the function selector 'SetTax(uint256') the encoded parameter of 'tax-ref'
Relating to my above comment(I was thinking about general implementations there, not hard coding options for proposals) We would want to think about building out a nested form for the calldata section that included the proposed input fields, titles, etc. From there just support logic to support step vi-vii from my above comment
Maybe in this case we give the option to bypass the contract link and add a tx_form
FunctionBuilderSelection: {
field_name: 'Calldata',
field_label: 'The Calldata to send to each Contract address',
field_ref: 'calldata-ref',
tx_forms: [
{ functionSelector: 'SetTax(uint256),
paramRefs: ['tax-ref']
form: [ {
"text": {
"field_name": "subtitle",
"field_type": "h2",
"field_value": "Parameter Change: Set Tax"
}
},
{
"input": {
"field_name": "tax man tax man",
"field_label": "Set the tax man",
"field_ref": "tax-ref",
"formatter": "number"
}
}
]
},
The idea here is that instead of having a dynamic contract_link to look up any ABI we can hard code and add formatting for a typical template like setTax. each object in 'form' will get hashed in to calldata and added to the call data list while the FE can use the form data to populate the CallData FunctionBuilderSelection. A functionBuilderSelector should always have a contract_link(dynamic to address) or a form(static to a typical proposal pattern) but not both.
Condensing/TL;DR of my thoughts above:
I think the 'easy win' here and maybe the short term solution is to:
This allows for templated spend, param, etc proposals
I'd say the first set of ideas I mentioned, where we dynamically pull abis for provided contracts could be a stretch goal but definitely valuable.
Thoughts @jnaviask @zakhap @CowMuon ?
https://github.com/hicommonwealth/commonwealth/commit/805d5a96a6b9212059647d7c310df4b5a6ba4515 prototype(logic only, not UI yet) of how we could handle the proposed schema
I think this is a fine solution. We may want to eventually consider exposing the signature of the function we're providing as calldata, because that's a required parameter on compound proposals.
What I would like to see is what schema modifications this requires -- we may want to also add a version to the schema in case we need to edit it further later.
A limitation with our proposal template schema has been identified.
Current Status:
Issue:
The problem arises when we have arguments within the
method
on the transaction template that depend on other parameters in the method, or on the contract itself.An example here is the propose method on the OpenZeppelin governor alpha contract. It expects an argument called
calldatas
. As can be seen in this test example, the value of calldatas can be the result of a function calledencodeParameters
, which takes anacct
as an argument. Since theacct
would be one of the pieces of form state (i.e., a "field_ref" in the proposal blob), we cannot simply pass its output as a string at template creation time. It needs to resolve dynamically based on user inputs in the parser page.So we need to rethink the schema slightly to address two problems:
@jnaviask @MuonShot @zakhap , something to think about before we decide to ship bet1. Seems to me that most of the value the bet provides is related to templates for actions like
propose
.Possible solution sketch:
'$CONTRACT_ADDRESS'
that the parser will just always replace with the contract address.'&myWhitelistedFn($abitrary-ref, $CONTRACT_ADDRESS, 'simple-value', ...)'
. The parser check if the function is in the registry and then call it with those inputs before formatting the tx. It can then be used in the tx_template just like the simple state refs are already.Also could allow them to pass an endpoint for a lambda that runs the specified function- have to think about the UX in that case.