-
Notifications
You must be signed in to change notification settings - Fork 53
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
refactor(svm): restructure utils into modular files under src folder #793
base: master
Are you sure you want to change the base?
refactor(svm): restructure utils into modular files under src folder #793
Conversation
…for better organization Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
…be-in-src-from-the-test-directory
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
Signed-off-by: Pablo Maldonado <[email protected]>
{ Property: "relayer", Value: new PublicKey(event.data.relayer).toString() }, | ||
{ Property: "depositor", Value: new PublicKey(event.data.depositor).toString() }, | ||
{ Property: "recipient", Value: new PublicKey(event.data.recipient).toString() }, | ||
{ Property: "message", Value: event.data.message.toString() }, |
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.
fill events don't have full message anymore. instead its messageHash
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.
ye, this will be broken now if used. I think it could be due to a merge issue you had.
Property: "updatedRecipient", | ||
Value: new PublicKey(event.data.relayExecutionInfo.updatedRecipient).toString(), | ||
}, | ||
{ Property: "updatedMessage", Value: event.data.relayExecutionInfo.updatedMessage.toString() }, |
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.
updatedMessageHash
@@ -36,7 +35,7 @@ const argv = yargs(hideBin(process.argv)) | |||
.option("inputAmount", { type: "number", demandOption: true, describe: "Input amount" }) | |||
.option("outputAmount", { type: "number", demandOption: true, describe: "Output amount" }) | |||
.option("originChainId", { type: "string", demandOption: true, describe: "Origin chain ID" }) | |||
.option("depositId", { type: "string", demandOption: true, describe: "Deposit ID" }) | |||
.option("depositId", { type: "array", demandOption: true, describe: "Deposit ID" }) |
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.
its too cumbersome to pass array in command line (need to repeat --depositId param for each byte. Better use string and parse it as hex (if it starts with 0x) or number
@@ -50,7 +49,7 @@ async function fillV3Relay(): Promise<void> { | |||
const inputAmount = new BN(resolvedArgv.inputAmount); | |||
const outputAmount = new BN(resolvedArgv.outputAmount); | |||
const originChainId = new BN(resolvedArgv.originChainId); | |||
const depositId = intToU8Array32(new BN(resolvedArgv.depositId)); | |||
const depositId = resolvedArgv.depositId; |
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.
why change this
console.log("Fill events fetched successfully:"); | ||
fillEvents.forEach((event, index) => { | ||
console.log(`Fill Event ${index + 1}:`); | ||
console.table([ | ||
{ Property: "inputToken", Value: strPublicKey(event.data.inputToken) }, | ||
{ Property: "outputToken", Value: strPublicKey(event.data.outputToken) }, | ||
{ Property: "inputToken", Value: new PublicKey(event.data.inputToken).toString() }, |
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.
why drop strPublicKey helper, we introduced this recently
@@ -65,7 +64,7 @@ async function fillV3Relay(): Promise<void> { | |||
inputAmount, | |||
outputAmount, | |||
originChainId, | |||
depositId, | |||
depositId: depositId.map((id) => Number(id)), |
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 might be based on old version. we had it fixed recently
@@ -155,7 +154,7 @@ async function fillV3Relay(): Promise<void> { | |||
state: statePda, | |||
signer: signer.publicKey, | |||
instructionParams: program.programId, | |||
mint: outputToken, | |||
mintAccount: outputToken, |
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.
should be mint
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.
same comment as before, likely due to a merge issue.
/** | ||
* Converts an integer to a 32-byte Uint8Array. | ||
*/ | ||
export function intToU8Array32(num: number): number[] { |
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.
we had this fn also handle BigInt numbers
/** | ||
* Converts an EVM address to a Solana PublicKey. | ||
*/ | ||
export const evmAddressToPublicKey = (address: string): PublicKey => { |
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 more belongs to conversionUtils
/** | ||
* Converts a Solana PublicKey to an EVM address. | ||
*/ | ||
export const publicKeyToEvmAddress = (publicKey: PublicKey | string): string => { |
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 more belongs to conversionUtils
}, | ||
}; | ||
|
||
private static coderTypes: IdlTypeDef[] = [ |
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 can all be linted a lot better. might be ok to ignore though and we can run a seperate linting task to collaps JS objects after the fact.
@@ -0,0 +1,6 @@ | |||
export * from "./relayHashUtils"; |
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 a great refactor.
Changes proposed in this PR: