Skip to content

Conversation

@Reinis-FRP
Copy link
Contributor

We had custom serialization implementation when hashing relayer refund and slow fill leaves. This just concatenated array elements without encoding array length. This PR fixes by using Anchor's borch serializer that encodes this more consistently.

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
for address in &self.refund_addresses {
bytes.extend_from_slice(address.as_ref());
}
AnchorSerialize::serialize(&self, &mut bytes)?;
Copy link
Contributor

@md0x md0x Nov 22, 2024

Choose a reason for hiding this comment

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

Wow! Much better like this

.remainingAccounts(fillRemainingAccounts)
.rpc();
.instruction();
const computeBudgetInstruction = ComputeBudgetProgram.setComputeUnitLimit({ units: 1_400_000 });
Copy link
Contributor

Choose a reason for hiding this comment

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

Has the borsh serialisation increased the CU?

Copy link
Contributor Author

@Reinis-FRP Reinis-FRP Nov 22, 2024

Choose a reason for hiding this comment

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

Its hard to tell precisely as our tests are not deterministic and have much more variance due to differing bumps in PDA derivation, but over 10 runs I observed on average +2% in CU, which I guess is acceptable here.


const refundAddressesBuffer = Buffer.concat(relayData.refundAddresses.map((address) => address.toBuffer()));

// TODO: We better consider reusing Borch serializer in production.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 we should probably test this and make sure they produce the same byte layout

Copy link
Member

@chrismaree chrismaree left a comment

Choose a reason for hiding this comment

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

this is great!

@Reinis-FRP Reinis-FRP merged commit 1573d00 into master Nov 23, 2024
9 checks passed
@Reinis-FRP Reinis-FRP deleted the reinis-frp/borch-serializer branch November 23, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants