Skip to content

fix: #60, Check if preimage exists on chain before creating a proposal #61

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.

Sign up for GitHub

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

Merged
mordamax merged 3 commits into from
May 23, 2023
Merged

fix: #60, Check if preimage exists on chain before creating a proposal #61

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44ac4ba
fix: #60, Check if preimage exists on chain before creating a proposal
mordamax May 18, 2023
4315da8
remove console.log
mordamax May 18, 2023
50aadc1
Fix length for Lookup
mordamax May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fix length for Lookup
  • Loading branch information
@mordamax
mordamax committed May 19, 2023
commit 50aadc113d5f6e3d3817f6ad869a0c58cead9640
3 changes: 2 additions & 1 deletion src/tip-opengov.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export async function tipOpenGov(opts: {
]);
const encodedProposal = proposalTx.method.toHex();
const proposalHash = blake2AsHex(encodedProposal);
const encodedLength = Math.ceil((encodedProposal.length - 2) / 2);
Copy link
Member

@ggwpez ggwpez May 20, 2023

Choose a reason for hiding this comment

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

What is this calculation? It needs to be the SCALE encoded length.

Copy link
Contributor Author

@mordamax mordamax May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

before it was proposalTx.length - 1, but this I took from https://github.com/polkadot-js/apps/blob/2d295e33f9d37d6582d97b9e93df81d16e1950e2/packages/page-preimages/src/Preimages/Add/Partial.tsx#L44

@ggwpez could you please explain what is SCALE encoded?

Copy link
Member

@ggwpez ggwpez May 22, 2023

Choose a reason for hiding this comment

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

It first does toHex and then uses (x - 2) / 2 to get the number of bytes in the hex string… wtf
Maybe there is no easier way to encode it or something. Ideal would be an encode or encodedLength function.
Otherwise just add a test; i dont want to block over this if it works.

SCALE is the encoding that Substrate/Polkadot uses for mostly everything. Especially stuff like transactions and their arguments (so also preimages).

Copy link
Contributor Author

@mordamax mordamax May 23, 2023

Choose a reason for hiding this comment

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

I'll investigate and we will add tests for sure


return await new Promise(async (resolve, reject) => {
// create a preimage from opengov with the encodedProposal above
Expand All @@ -57,7 +58,7 @@ export async function tipOpenGov(opts: {
.submit(
// TODO: There should be a way to set those types properly.
{ Origins: track.track } as never,
{ Lookup: { hash: proposalHash, length: proposalTx.length - 1 } },
{ Lookup: { hash: proposalHash, len: encodedLength } },
Copy link
Member

@ggwpez ggwpez May 20, 2023

Choose a reason for hiding this comment

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

Suggested change
{ Lookup: { hash: proposalHash, len: encodedLength } },
{ Lookup: { hash: proposalHash, len: readPreimage.len } },

Not sure if that is the correct syntax, but the len should be in the RequestStatus

image

You can also SCALE encode yourself or use the encoded len though.

Copy link
Contributor Author

@mordamax mordamax May 22, 2023

Choose a reason for hiding this comment

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

len: readPreimage.len there's no such field in the returned response, there's readPreimage.encodedLength but it returns 54, while expected number is 85

Here's some logs

proposalTx.encodedLength 88
proposalTx.toString {"signature":{"signer":{"id":"CaKWz5omakTK7ovp4m3koXrHyHb7NG3Nt7GENHbviByZpKp"},"signature":{"ed25519":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"},"era":{"immortalEra":"0x00"},"nonce":0,"tip":0},"method":{"callIndex":"0x1800","args":{"calls":[{"callIndex":"0x0000","args":{"remark":"0x544f3a206d6f7264616d617820464f523a206d6567617265706f2331342028736d616c6c29"}},{"callIndex":"0x1203","args":{"amount":2000000000000,"beneficiary":{"id":"HnMAUz7r2G8G3hB27SYNyit5aJmh2a5P4eMdDtACtMFDbam"}}}]}}}
proposalTx.length 86
encodedProposal 0x180008000094544f3a206d6f7264616d617820464f523a206d6567617265706f2331342028736d616c6c2912030b00204aa9d10100e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e
encodedProposal.length 172
readPreimage.encodedLength 54 (¯\\_(ツ)_/¯)
encodedLength 85

when I create it, in polkadotjs it shows 85

image

Copy link
Contributor Author

@mordamax mordamax May 22, 2023

Choose a reason for hiding this comment

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

I just added 5 chars to a remark, and got 90 as final size in polkadotjs

so it means to me

proposalTx.encodedLength - 3
proposalTx.length - 1
Math.ceil((encodedProposal.length - 2) / 2)

all 3 will give same correct size of preimage

https://github.com/search?q=+%22Math.ceil%28%28encodedProposal.length+-+2%29+%2F+2%29%3B%22&type=code

I just used last as this is most commonly used

Copy link
Member

@ggwpez ggwpez May 22, 2023

Choose a reason for hiding this comment

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

Okay yea then just use what works. The TS code is fucked either way.

{ after: 10 } as never,
)
.signAndSend(botTipAccount, { nonce: -1 }, async (refResult) => {
Expand Down