-
Notifications
You must be signed in to change notification settings - Fork 8
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
Inline the preimages in the referendum #100
Conversation
rzadp
commented
Jul 6, 2023
- Closes Inline preimages (if possible) #76
- Closes Clean up preimages after referendum is complete #73 - no preimages, no cleanup necessary
- Closes Add tests for opengov & preimage size #65 - no preimages to test, also we have an E2E tests that makes sure that everything is constructed properly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good.
I have one curious question, but it fulfills the requirement of the ticket
@@ -133,3 +133,6 @@ export const formatTipSize = (tipRequest: TipRequest): string => { | |||
* It is used to tag these usernames when there is a failure. | |||
*/ | |||
export const teamMatrixHandles = ["@przemek", "@mak", "@yuri", "@bullrich"]; | |||
|
|||
// https://stackoverflow.com/a/52254083 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
const proposalByteSize = byteSize(encodedProposal); | ||
if (proposalByteSize >= 128) { | ||
return { | ||
success: false, | ||
errorMessage: `The proposal length of ${proposalByteSize} equals or exceeds 128 bytes and cannot be inlined in the referendum.`, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that the requirement of #76 is to only handle proposal smaller than 128 bytes.
Tho, out of curiosity, could you explain it to me please? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The runtime has some hardcoded limit on the proposal size (I tried inlining a bigger proposal and the extrinsic was rejected).
We don't expect our proposal size to vary - every proposal is the same - a call to treasury.spend
, and the only difference are the parameters of that call.
So I expect all our proposal to be <128 bytes, and I put this if
here as an assertion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification
// create a preimage from opengov with the encodedProposal above | ||
const preimageUnsubscribe = await api.tx.preimage | ||
.notePreimage(encodedProposal) | ||
.signAndSend(botTipAccount, async (result) => { | ||
await signAndSendCallback(bot, contributor.account, "preimage", preimageUnsubscribe, result) | ||
.then(async () => { | ||
const readPreimage = await api.query.preimage.statusFor(proposalHash); | ||
|
||
if (readPreimage.isEmpty) { | ||
reject(new Error(`Preimage for ${proposalHash} was not found, check if the bot has enough funds.`)); | ||
} | ||
|
||
const proposalUnsubscribe = await api.tx.referenda | ||
.submit( | ||
// TODO: There should be a way to set those types properly. | ||
{ Origins: track.track.trackName } as never, | ||
{ Lookup: { hash: proposalHash, len: encodedLength } }, | ||
{ after: 10 } as never, | ||
) | ||
.signAndSend(botTipAccount, async (refResult) => { | ||
await signAndSendCallback(bot, contributor.account, "referendum", proposalUnsubscribe, refResult) | ||
.then(resolve) | ||
.catch(reject); | ||
}); | ||
}) | ||
const proposalUnsubscribe = await api.tx.referenda | ||
.submit( | ||
// TODO: There should be a way to set those types properly. | ||
{ Origins: track.track.trackName } as never, | ||
{ Inline: encodedProposal }, | ||
{ after: 10 } as never, | ||
) | ||
.signAndSend(botTipAccount, async (refResult) => { | ||
await signAndSendCallback(bot, contributor.account, "referendum", proposalUnsubscribe, refResult) | ||
.then(resolve) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is removed because you are already verifying the size of the proposal, right?
As per #76:
If the preimage is <128 byte, then you can Inline it. So you dont need an additional TX just to register it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, at this point we verified the size and go straight to inlining, we can forget about the additional TX.