Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Keychain fees should not be paid in advance #561

Closed
Pitasi opened this issue Jul 26, 2024 · 0 comments · Fixed by #662
Closed

Keychain fees should not be paid in advance #561

Pitasi opened this issue Jul 26, 2024 · 0 comments · Fixed by #662
Assignees
@backsapc
Labels
protocol

Comments

@Pitasi
Copy link
Contributor

Pitasi commented Jul 26, 2024

The current flow is:

  • Keychain operator sets a certain fee for its services (i.e. key and signature generation)
  • when user requests a new key or signature, the fee is deducted from their balance
  • the keychain then can fulfill the request, but also go offline or reject it
  • the fees are never returned to the user in case the service is not delivered succesfully

What we should have instead, is warden acting as an escrow:

The user pays for the service to the protocol (an escrow account), and only when the keychain fulfills the request they receive the fees. If the keychain rejects the request or after a certain timeout, the fees are refunded to the user.
@backsapc backsapc self-assigned this Aug 9, 2024
@backsapc backsapc mentioned this issue Aug 12, 2024
Merged
@backsapc backsapc linked a pull request Aug 20, 2024 that will close this issue
#561 Keychain fees should not be paid in advance #662
Merged
backsapc added a commit that referenced this issue Aug 30, 2024
@backsapc @Pitasi @coderabbitai
#561 Keychain fees should not be paid in advance (#662)
09828be 
* Added keychain max wanted fees handling

* Implemented new testcase & snapshot

* Added annotations

* Added keychain fee max value specification

* Code review fixes

* Changed validation messages

* fix(x/act): add support for sdk.Coins in CLI flags

* Changed type to sdk.Coin

* Updated CHANGELOG.md

* Implemented escrow accounts for keychain fees

* Added CHANGELOG.md line

* Regenerated keychain snapshot

* Updated keychain deduction fee code

* Added test case for fee deduction with --max-keychain-fee

* Fixed actions auto parsing for cli

* Fixed bug with not forking fees deduction and context missing values

* Docker versions update

* Update cmd/wardend/cmd/gen-keychains.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Proto rebuild

* Merged changes from #560

* Code-review fixes

* Implemented integration test for escrow account

* Code review fixes

* Made KeychainFees = nil invalid

* Fixed error message

* Keychain fee validations

* Code review fixes

* Merge conflict fixes

* Lint fixes

---------

Co-authored-by: Antonio Pitasi <antonio@pitasi.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
mn13 pushed a commit that referenced this issue Sep 3, 2024
@backsapc @Pitasi
@coderabbitai @mn13
#561 Keychain fees should not be paid in advance (#662)
868724a 
* Added keychain max wanted fees handling

* Implemented new testcase & snapshot

* Added annotations

* Added keychain fee max value specification

* Code review fixes

* Changed validation messages

* fix(x/act): add support for sdk.Coins in CLI flags

* Changed type to sdk.Coin

* Updated CHANGELOG.md

* Implemented escrow accounts for keychain fees

* Added CHANGELOG.md line

* Regenerated keychain snapshot

* Updated keychain deduction fee code

* Added test case for fee deduction with --max-keychain-fee

* Fixed actions auto parsing for cli

* Fixed bug with not forking fees deduction and context missing values

* Docker versions update

* Update cmd/wardend/cmd/gen-keychains.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Proto rebuild

* Merged changes from #560

* Code-review fixes

* Implemented integration test for escrow account

* Code review fixes

* Made KeychainFees = nil invalid

* Fixed error message

* Keychain fee validations

* Code review fixes

* Merge conflict fixes

* Lint fixes

---------

Co-authored-by: Antonio Pitasi <antonio@pitasi.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
mn13 added a commit that referenced this issue Sep 5, 2024
@mn13 @backsapc
@artur-abliazimov @jlehtimaki @jjheywood @Pitasi @alijnmerchant21 @ijonele @alex-nax @coderabbitai @laniakea42 @technicallyty
fix(wardend): validate bech32 addresses in add-genesis-keychain and a…
fa02342 
…dd-genesis-space (#723)

* validate bench32 in cmd
add-genesis-keychain
add-genesis-space

* in executeAnalyzers return error on bad contract address (#716)

* in executeAnalyzers return error on bad contract address

* update changelog

* validate bench32 in cmd
add-genesis-keychain
add-genesis-space

* fix spelling and duplicates

* * Added KeybaseId, Url and Name fields to keychain

* Added changelog

* Review comments

* Code review fixes

* Regenerated model

---------

Co-authored-by: Archie <artur.abliazimov@gmail.com>

* Fix of localnet.just (#726)

* Fixed localnet.just

* Fixed localnet.just

* Remove the GMP default params from genesis (#727)

* feat(faucet): new faucet (#479)

* feat(faucet): new faucet

This version of the Faucet does not have any limitations per user, only
limitations are daily token supply

* fix: minor css changes for mobile

* fix(faucet): add daily refresh and token refresh job

* fix(faucet): fix bugs and ci

* fix(faucet): remove unnessary go.mod files, replace dockerfile faucet-v2

* chore(faucet): remove unnessary variables and modify types

this will modify the types of DailyLimit and Batchinterval to remove
unnessary conversions and remove Cooldown and BatchInterval

* fix(faucet): acquire a lock when setting the new supply

* fix(faucet): removed locking where it's not needed

other enhancements as well, the loading and setting context to correct
place

* fix(faucet): converted background to much smaller image

* feat(faucet): make the run out of daily nicer

* feat(faucet): add new style

* feat(faucet): add check that same address cannot be multiple time in one batch

* feat(faucet): add metrics

* fix(faucet): Dockerfile libwasmvm was missing access rights

* feat(faucet): add pre input option to address form

---------

Co-authored-by: Jon Heywood <94459819+jjheywood@users.noreply.github.com>

* chore(wardend): bump ibc-go to v8.4.0

* docs: Update Keychain SDK (#706)

* Upgrade keychain docs

Update go version

Update json for keychain-fees

Update chain-id

* Add categorization to Keychain SDK

* Categorization

* Add Keychain SDK + Description

* Create a new tutorial

Add basic info for keychain

Implement main func with - handleKeyRequest and handleSignRequest

Write test function

* fixed a link in the Tools menu

* quick fixes

* quick fixes

* quick fixes

* remove )

---------

Co-authored-by: Margarita Skomorokh <ijonele@gmail.com>

* fixed links to the keychain sdk (#733)

* Made KeychainFees = nil invalid

* ci: add check to ensure go.mod is tidy

* chore: go mod tidy

* fix(wardend): ICAHostKeeper initialization requires QueryRouter to be set

Breaking change introduced in ibc-go v8.3

* feat(spaceward): key creation in sidebar (#728)

* fix key icon in sidebar

cleanup

* fix getProider call

* bump ethers version; fix erc20 send

* build: use new justfile commands

* fix(x/warden): make KeychainFees non-nullable

* fix(x/warden): allow empty KeychainFees

* cosmoshubtestnet hotfix; do not fail on wrong message type (#746)

* feat(faucet): configurable chain name (#747)

this change adds configurable chain/faucet name to the site

* fix(faucet): chain variable missing from struct (#749)

This PR adds the missing Chain variable that missed in #747

* #561 Keychain fees should not be paid in advance (#662)

* Added keychain max wanted fees handling

* Implemented new testcase & snapshot

* Added annotations

* Added keychain fee max value specification

* Code review fixes

* Changed validation messages

* fix(x/act): add support for sdk.Coins in CLI flags

* Changed type to sdk.Coin

* Updated CHANGELOG.md

* Implemented escrow accounts for keychain fees

* Added CHANGELOG.md line

* Regenerated keychain snapshot

* Updated keychain deduction fee code

* Added test case for fee deduction with --max-keychain-fee

* Fixed actions auto parsing for cli

* Fixed bug with not forking fees deduction and context missing values

* Docker versions update

* Update cmd/wardend/cmd/gen-keychains.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Proto rebuild

* Merged changes from #560

* Code-review fixes

* Implemented integration test for escrow account

* Code review fixes

* Made KeychainFees = nil invalid

* Fixed error message

* Keychain fee validations

* Code review fixes

* Merge conflict fixes

* Lint fixes

---------

Co-authored-by: Antonio Pitasi <antonio@pitasi.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* feat(spaceward): update for rebrand (#748)

* feat(spaceward): New theme added

* fix(spaceward): Some theme fixes

* fix(spaceward): Fixed keychains

* fix(spaceward): Some theme fixes and metamask

* fix(spaceward): Some more

* fix(spaceward): Space Mono

* fix(spaceward): fix for space mono font issue (#750)

* fix(spaceward): updated welcome message (#751)

* fix(faucet): fix daily limit i64 (#755)

This fixes issue where user puts high uward amount to daily limit and go
cannot parse it due to int restrictions

* Add chain upgrade instructions for v0.4.2 (#757)

* Add chain upgrade instructions for v0.4.2

* Update v0.4.2.md

* update v0.4.2

Co-authored-by: Joonas Lehtimäki <joonas.lehtimaki@gmail.com>

* remove reference to side-car

---------

Co-authored-by: Joonas Lehtimäki <joonas.lehtimaki@gmail.com>

* fixed spelling (#758)

* fixed spelling

* deleted the tip, fixed spelling

* update slinky

* fix(x/act): simplify prepareHandlerContext to always use sdk.Context

* updated gov proto; fix governance types (#761)

* Fix returned field (#765)

* fix(spaceward): Fix logos (#767)

* fix(spaceward): Fix logos

* fix(spaceward): updated logo for WC connection and updated hover fill colour

---------

Co-authored-by: Jon Heywood <94459819+jjheywood@users.noreply.github.com>

* fix(faucet): tx page loading (#768)

* feat(faucet): add new metric dailySupply

* fix(faucet): page keeps loading even if tx was finished

---------

Co-authored-by: backsapc <backsapce@hotmail.com>
Co-authored-by: Archie <artur.abliazimov@gmail.com>
Co-authored-by: Joonas Lehtimäki <joonas.lehtimaki@gmail.com>
Co-authored-by: Jon Heywood <94459819+jjheywood@users.noreply.github.com>
Co-authored-by: Antonio Pitasi <antonio@pitasi.dev>
Co-authored-by: Aliasgar Merchant <44069404+alijnmerchant21@users.noreply.github.com>
Co-authored-by: Margarita Skomorokh <ijonele@gmail.com>
Co-authored-by: Margarita Skomorokh <margaret.skomorokh@anychart.com>
Co-authored-by: Alex D <alex.d.nax@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Alice <37526212+laniakea42@users.noreply.github.com>
Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>
@coderabbitai coderabbitai bot mentioned this issue Oct 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@backsapc backsapc

Labels
protocol
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#561 Keychain fees should not be paid in advance warden-protocol/wardenprotocol
2 participants
@Pitasi @backsapc