Skip to content

Conversation

@Ryang-21
Copy link
Collaborator

No description provided.

@Ryang-21 Ryang-21 requested a review from mootz12 April 14, 2025 14:45
Copy link
Contributor

@mootz12 mootz12 left a comment

Choose a reason for hiding this comment

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

Looks good - just some feedback around allowance creation

Comment on lines 299 to 315
// Check if the allowance is less than u64 max
if (
amount < BigInt('18446744073709551615') / BigInt(2) ||
expiration < allowance.currLedger + 17368 * 7
) {
const assetContract = new Contract(allowance.assetId);
const op = assetContract
.call(
'approve',
...[
Address.fromString(allowance.filler.keypair.publicKey()).toScVal(),
Address.fromString(allowance.spender).toScVal(),
nativeToScVal(BigInt('18446744073709551615'), { type: 'i128' }),
nativeToScVal(allowance.currLedger + 17368 * 30 * 5, { type: 'u32' }),
]
)
.toXDR('base64');
Copy link
Contributor

Choose a reason for hiding this comment

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

this will submit a lot of unnecessary allowance calls (IE - after each successful fill), burning unnecessary XLM.

maybe just check if its under like BigInt(100_000 * 1e7), and continue making the allowance u64::MAX

const ledgersToFill = auctionEntry.fill_block - nextLedger;
if (auctionEntry.fill_block === 0 || ledgersToFill <= 5 || ledgersToFill % 10 === 0) {
// Check is the filler has an active allowance for backstop token
if (auctionEntry.auction_type === AuctionType.Interest) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we only want to send this once when fill_block === 0? The buffer in expiration_ledger ensures the allowance will exist over the next day of blocks, so there is no risk of it expiring beforehand

Comment on lines 294 to 298
const expiration = await sorobanHelper.loadAllowanceExpiration(
allowance.assetId,
allowance.filler.keypair.publicKey(),
allowance.spender
);
Copy link
Contributor

Choose a reason for hiding this comment

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

should this just load the allowance object in full? We are only interacting with the Comet contract, and we need to load the ledger entry from chain anyway for the expiration_ledger

src/events.ts Outdated
POOL_EVENT = 'pool_event',
USER_REFRESH = 'user_refresh',
CHECK_USER = 'check_user',
CHECK_ALLOWANCES = 'check_allowances',
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used?

Copy link
Contributor

@mootz12 mootz12 left a comment

Choose a reason for hiding this comment

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

LG2M

@Ryang-21 Ryang-21 merged commit 002c568 into main Apr 24, 2025
1 check passed
@Ryang-21 Ryang-21 deleted the v2 branch April 24, 2025 15:53
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.

3 participants