iangriggs / near_delegation

0 stars 0 forks source link

Unable to use Near Function Access Key to call a payable contract function that needs a deposit #1

Open iangriggs opened 2 years ago

iangriggs commented 2 years ago

I was hoping to take advantage of Near function scoped access keys to perform a delegated contract function call against a 3rd party contract but ran into either a DepositWithFunctionCall error message or a contract panic when no deposit specified.

Diagram of implementation: image

The contract in question is a 3rd party contract (actually Mintbase fyi) so tweaking contract out of my control. The grant_minter function in question is marked as payable. It also seems to require a deposit of 1 yoctoNear otherwise it returns panicked: Requires attached deposit of exactly 1 yoctoNEAR

The logic within the contracts grant_minter function asserts that is is being called by a designated contract owner account i.e. in the diagram it is represented by admin.testnet. I'm hoping to allow normal users to call this function but only via a restricted Function Access Key.

The idea was that a normal user user1.testnet calls a middleware function /grant_minter that passes on the call to the /grant_minter contract function. The middleware function calls with the identity of admin.testnet by creating a restricted Function Access Key for admin.testnet from the private key held in a .env environment variable. This therefore would allow the contract to be effectively called by an admin.testnet account instance with a very restricted access key.

Unfortunately I ran into the DepositWithFunctionCall error message when the necessary deposit was specified as this seems to be disallowed by design.

I can make this work by loading a Full Access key in the middleware for admin.testnet but that is obviously a very bad idea and I don't want to do that.

In this user-case it seems surprising to restrict the benefits of a Function Access Key when passing a deposit? I'm sure there is a good reason but for my use-case feels valid?

Is there another way of achieving the aim of my implementation? I've see the videos on Live App Review and another on Multi-Sig and feels like there might be a trick in achieving this. The alternative is for me to manually call the grant_minter function on request by a user but that feels a clunky Ux.

mattlockyer commented 2 years ago

It's true that limited access keys do not allow deposits by design.

This is because apps typically hold the private keys for these keys e.g. sign in with NEAR Wallet typically creates a key for that app.

You can prompt the user to call a contract method with a deposit from a connected wallet and they will be redirect to the NEAR wallet to sign the TX using their full access key (since NEAR Wallet holds private keys in localStorage).

Why is it such a bad idea to have full access key in middleware? Provided you can store safely? Is there something about the architecture that would leak this key to others? Are you afraid of someone finding exploit in middleware and using key to drain funds?

Personally, I would take a 2 step process (think of middleware as escrow account).

  1. User sends funds to contract to cover expenses. Contract stores account -> balance mapping.
  2. Give user limited access key that allows them to only call /grant_minter and use their balance to make a cross contract call to Mintbase /grant_minter

Not sure if this is solving your problem, I feel like I'm missing some information about problem.

iangriggs commented 2 years ago

Thanks for thinking though that Matt.

From what you are saying it sounds like my idea of using a limited access key from the middleware I've described is a no go so I guess I need to accept that. Still not sure of the reasoning behind limiting that but I'm sure it's is a good one πŸ˜€

It's not that I don't trust the security of the middleware. It just felt like an unnecessary risk if I had the luxury of delegating to a limited access key.

In this use-case I'm actually not too worried about the user paying for this function call. I was happy for the admin account funding it so no real need for an escrow account or any wallet interaction. An interesting thought though.

Just to add a bit more information to the problem...

The function in the 3rd party Mintbase contract I'm calling is this: https://github.com/Mintbase/mintbase-core/blob/2cab87eee5f7a1d47a232210c2224ced4be7d724/mintbase-deps/src/impls.rs#L1222

You'll notice it has an assert self.assert_store_owner();. In my description above the store owner is my imaginary admin.testnet account. The assert_store_owner is obviously intended to stop accounts that aren't a store owner from calling them. I'm trying to avoid myself, the store owner, needing to call the grant_minter account as a manual operation as that feels like clunky Ux to on-board a new user signing up and becoming a minter. Hence the attempt to delegate a normal user request, via the middleware, to effectively use admin access key to call grant_minter.

I've proven it is possible with a full access key so I guess I just need to decide my level of confidence in keeping what is the main store owner private key in an environment variable with the middleware. 😬

Alternatively, a multi-sig arrangement where the user kicks off the process and then I receive some form of notification to provide the second configuration signature sounds pretty neat Ux (albeit still requiring me to be manually involved in the flow)...and potentially makes it possible to let the user pay the storage fee as you suggested.

Hope that adds a bit more clarity to the scenario I working on.

mattlockyer commented 2 years ago

Still not sure of the reasoning behind limiting that but I'm sure it's is a good one

It would leave apps open to spending the users NEAR indiscriminately, e.g. transferring it to themselves vs. spending the limited access key allowance ONLY on gas for transactions.

Hence the attempt to delegate a normal user request, via the middleware, to effectively use admin access key to call grant_minter.

You should deploy a proxy contract on the account of admin.testnet that allows the user to call grant_minter via a cross contract call to the mintbase contract.

image

You don't really need to manage middleware, you just need to determine WHO your user is. HOW did they get an access key to call admin.testnet and whether or not they are currently still "valid" to call grant_minter.

iangriggs commented 2 years ago

Thanks again.

I think I'm now seeing the reasoning for the limitation. It still feels like a cool thing to allow if the downside you described could be guarded against. The aspect I really liked about NEAR was the on-boarding flow for non-NEAR holders, the 'skin in the game' approach I think you termed it in on of your videos. If there are calls that require a deposit I guess this can still be achieve with an escrow techniques or the cross-contract call so achievable in other ways. I'll leave that discussion there now and accept it's by design πŸ‘

The proxy contract sounds like the trick/technique I was looking for as a solution. Looks like you're going to force me to learn rust and ruin my original tactic of using the mintbase nft contracts to avoid that. πŸ˜€ ...no bad thing and on my todo list. I've already spotted a video with a familiar face on it (https://www.youtube.com/watch?v=971dTz6nM2g) on the topic of cross-contract calls.

Is it the case with NEAR (looking at your proxy contract diagram above) that the grant_minter call from admin-testnet proxy contract to mintbase contract appears to mintbase contract to be from the account of admin.test rather than the user account. Does this relate to the topic of sender vs predecessor I vaguely remember mentioned in a separate video?

I'm learning more and more through this...thanks again!

mattlockyer commented 2 years ago

Is it the case with NEAR (looking at your proxy contract diagram above) that the grant_minter call from admin-testnet proxy contract to mintbase contract appears to mintbase contract to be from the account of admin.test rather than the user account.

yup.

see their method here it checks predecessor which is the last account in the chain of calls. https://github.com/Mintbase/mintbase-core/blob/2cab87eee5f7a1d47a232210c2224ced4be7d724/mintbase-deps/src/impls.rs#L1309-L1316

Sender A Predecessor B Current C Contract call ->

In a normal contract call sender and predecessor are the same A, B -> C

In a cross contract call, sender is always A, predecessor to B is A, predecessor to C is B A -> B -> C

A proxy contract shouldn't be too hard to deploy on the account and kind of fun! You can look for lots of examples here: https://github.com/near/near-sdk-rs Search for "ext_contract"

More resources: https://docs.near.org/docs/tutorials/contracts/xcc-receipts

BenKurrek commented 2 years ago

Interesting discussion. @mattlockyer wouldn't deploying a proxy contract not work in this case since the mintbase contract checks that the predecessor is the store owner? Unless you're suggesting deploying a proxy contract onto the store owner's account by which users then call a non-payable method on the store owner's newly deployed contract which will allow for a function call access key to be used to sign the txn. The store owner proxy contract will then perform a cross-contract call to the mintbase contract and attach the necessary 1 yoctoNEAR while also satisfying the requirement that the predecessor was the store owner.

mattlockyer commented 2 years ago

I was suggesting they deploy the proxy to the store owner account yes.

It seems like the problem to solve is how to open store ownership up to many users, without requiring them to create their own stores.

So turning the store owner into some sort of proxy/DAO contract account should work.

iangriggs commented 2 years ago

Hey @mattlockyer, thanks for the great info. I'll start exploring those links. Good spot on the Mintbase contract assert_store_owner function checking predecessor!

Hi @BenKurrek. Appreciate you taking the time to sanity check Matts great suggestion.

...and yes, the high-level problem is to allow an Mintbase NFT Store to onboard new NFT minter accounts to the Mintbase store contract without requiring the store owner account to manually add them via the front-end (currently a manual operation on the Mintbase frontend).

I've got plenty to work on now. Really appreciate all the help.

iangriggs commented 2 years ago

Hi @mattlockyer, here are some updates...

Following your advice I've successfully created a contract that performs a contract to contract call and from the NEAR CLI I was able to successfully called the grant_minter function from an account that isn't the contract owner account. This was a major milestone and I've added the contract source lib.rs to this repo if you'd be interested in casting your eye over it. As you said...not too tricky and was a nice challenge, although not 100% sure I've handled the gas param in a good way?

The next challenge was calling this from my frontend app. I soon ran into issues because the client app is signed into, and only has a function access key for, the mintbase.testnet contract. At this point the calling account has no function access key to my new contract performing the grant minter delegation so I naturally threw errors to that effect.

image

So, I've spent the majority of today looking through example code for adding function access keys but could find one that showed me cleanly how to add a new function access key without involving the wallet. Is it therefore correct to make use of the walletAccount.requestSignIn() function and pass suitable parameters to force a redirect to the wallet and it will add the necessary function access key to the calling account. It seems to work other than it results in a key with a pending key name in local storage?!

I can see from the wallet source code that if I wasn't signed in it would perform some local storage key renaming to fix-up the situation via _completeSignInWithAccessKey-> _moveKeyFromTempToPermanent(. Nevertheless, just to ensure my sanity that I was going in the right direction I renamed keys appropriately in local storage to test out the theory and can see the grant_minter call would actually complete successfully from the client

So, am I doing this the right way? Does it makes sense that I'm calling walletAccount.requestSignIn even though I'm essentially already signed in via a separate contract?

As you can hopefully see, I'm really close to getting the end to end flow working...just keen to know that what I'm now doing to get function access keys correctly assigned to the calling accounts it correct and there isn't a more elegant and recommended approach?

TIA for any further help.

iangriggs commented 2 years ago

Right, I've made more progress now and I believe I've overcome a fundamental misunderstanding I had regarding the way access keys relate to an account....

Up until now I've been thinking about them as giving access/permission to be able to call a contract. What I've realised from the many articles and youtube videos I've watched is that, the access keys should be considered the permissions the user is allowing a dapp to use to access a contract. It's more about protecting the user and their account than allowing access to a contract. Thinking about accounts an access keys in this way really helps understand why function access keys are preferred.

I've also realised that my ramblings above regarding calling requestSignIn whilst already signed in are also not required. As a result of using a 3rd party SDK (Mintbase) that wraps the Near JS SDK I wasn't passing in the correct account reference when setting up a new contract reference with new Contact(.... As a result I wasn't receiving a walletConnection object which IS able to retrieve an access key from my account (it a full access key which I guess is a not 100% ideal but hey-ho) but it avoids the nasty hacky code I had in place dealing with pending keys in local storage after calling requestSignin.

So, I've currently got a working solution and this issue could effectively be closed...but I'll leave it open for the time being. Hope it's useful for anyone who finds it and battles with the same issues and misunderstandings I had.