-
Notifications
You must be signed in to change notification settings - Fork 7
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: #60, Check if preimage exists on chain before creating a proposal #61
Conversation
]); | ||
const encodedProposal = proposalTx.method.toHex(); | ||
const proposalHash = blake2AsHex(encodedProposal); | ||
const encodedLength = Math.ceil((encodedProposal.length - 2) / 2); |
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.
What is this calculation? It needs to be the SCALE encoded length.
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.
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?
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.
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).
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'll investigate and we will add tests for sure
.submit( | ||
// TODO: There should be a way to set those types properly. | ||
{ Origins: track.track } as never, | ||
{ Lookup: { hash: proposalHash, len: encodedLength } }, |
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.
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.
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
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 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
I just used last as this is most commonly used
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.
Okay yea then just use what works. The TS code is fucked either way.
referenda_unsub(); | ||
} | ||
}); | ||
if (readPreimage.isEmpty) { |
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.
Asking for understanding:
At this point, we made sure that the image submission transaction is isInBlock
AND isFinalized
.
How can it be that it's empty then?
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.
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.
here i have 0,4 UNIT transferable, but for creating a preimage i will need 1,3
this is what happens with creating a preimage
In logs though all good, finalized, we proceeding to craetee a proposal. But there's no preimage. So it ends up with a proposal, without an "action" to execute after it's Voted and enaction time comes
fix: #60, Check if preimage exists on chain before creating a proposal
First submit a preimage and wait until it's finalized in blockHash
then call
await api.query.preimage.statusFor(proposalHash);
so we can make sure the preimage is therethen only create a proposal with this preimage
Bonus: fixed wrong field in proposal length -> len in Lookup, as currently it sends nothing (thus defaults to 0) which makes proposals not executable