Skip to content

feat(SessionKeyRegistry.sol): add deposit-n-rebate - #9

Open
DarkLord017 wants to merge 1 commit into
FilOzone:mainfrom
DarkLord017:feat/#7
Open

feat(SessionKeyRegistry.sol): add deposit-n-rebate#9
DarkLord017 wants to merge 1 commit into
FilOzone:mainfrom
DarkLord017:feat/#7

Conversation

@DarkLord017

Copy link
Copy Markdown

Will solve #7

  • Introduced deposit requirements for each permission slot (SLOT_DEPOSIT) and added logic to enforce minimum and maximum expiry times for permissions in SessionKeyRegistry.sol. Deposits are now tracked per permission using the new PermissionData struct.

  • Added new functions: clearExpired to remove expired permissions and refund deposits, and withdrawRefund to allow users to withdraw refundable deposits. Refund logic is now handled via the refundAmount mapping and emits events for deposit addition and refunds.

  • I'm still unsure if that's what was expected like I'm not sure about like how to decide on time limit on expiry and how much should a user deposit

  • Also should i also add max permission length to prevent out-of-gas issues

  • I have used the same mapping to include deposit amount also to reduce more storage

  • Also how should we handle duplication like currently it just adds that deposited amount also if expiry time is higher and rebates all the amount when the expiry passed is it correct ?

If my implmentation looks fine i will start working on tests

Pls review @wjmelements

@rvaggrvagg left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this has to be on hold for now, it's not in scope for GA so would rather not merge it until we're ready to start making changes in here

@wjmelements

Copy link
Copy Markdown
Collaborator

this has to be on hold for now, it's not in scope for GA so would rather not merge it until we're ready to start making changes in here

I disagree because the GA contracts are already deployed and frozen. This repo is not an archive so it is fine to enhance it.

@rvagg

Copy link
Copy Markdown

Well, I'd rather we not be making our lives harder by adding confusion just yet and iterating on the contracts where we don't plan on deploying those iterations pre-GA. Yes git tags solve for this, but let's avoid footguns where we can and keep things simple. I don't think we should have encouraged this work at this stage.

@BigLepBigLep added this to FOCMay 22, 2026
@github-project-automationgithub-project-automationBot moved this to 📌 Triage in FOCMay 22, 2026
@BigLepBigLep moved this from 📌 Triage to 🐱 Todo in FOCMay 22, 2026
@github-project-automationgithub-project-automationBot moved this from 🐱 Todo to ⌨️ In Progress in FOCMay 22, 2026
@BigLepBigLep moved this from ⌨️ In Progress to 🐱 Todo in FOCJun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🐱 Todo

Development

Successfully merging this pull request may close these issues.

4 participants

@DarkLord017@wjmelements@rvagg@BigLep