adding experiment with self calls #401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bowenwang1996
commented
Dec 13, 2023
Terraform Feature Environment (dev-401)Terraform Initialization ⚙️
|
encody
reviewed
Dec 15, 2023
contract/src/lib.rs
Outdated
| None => { | ||
| env::log_str("not ready"); | ||
| let account_id = env::current_account_id(); | ||
| PromiseOrValue::Promise(Self::ext(account_id).sign_helper(payload_id)) |
There was a problem hiding this comment.
Would it be possible to extend the delay for longer, for cheaper, by chaining 1-yocto transfers before the reflexive call, especially if a minimum delay is expected?
There was a problem hiding this comment.
How would chaining 1-yotco transfers help?
There was a problem hiding this comment.
I think the assumption here is that 1-yocto transfer is cheaper than a cross-contract call
There was a problem hiding this comment.
Maybe I am missing something here but I think you still have to call self again in order to continuously check whether a signature has been generated.
There was a problem hiding this comment.
Yeah, so I think what @encody means is that we could explore whether it makes sense to chain 10 1-yocto transfers first to create cheap superficial delay since we know that the response is not going to arrive in the first ~10 seconds (or whatever constant makes sense here), and then chain self-calls afterward expecting the response to have potentially come in by then.
|
|
|
Never mind the above, I figured out what the problem was. I just wasn't indexing txs that return receipt ids properly. From what I am seeing so far the approach works. I am getting a ton of timeout errors, but I think most of them are happening due to underlying tooling that relies on synchronous calls. Once I migrate everything to use asynchronous API I can confirm whether this works as reliably as expected. |
Closed
|
I have deployed this branch to multichain0.testnet and I can confirm that it works as expected. You can try it out for yourself by calling: Example tx: https://testnet.nearblocks.io/txns/3wJaSDxNZMPuZkzords3eEK3PkcHMnhtffQWP2cPPET5#execution |
encody
reviewed
Jan 16, 2024
contract/src/lib.rs
Outdated
| pub fn sign(&mut self, payload: [u8; 32], path: String) -> [u8; 32] { | ||
| near_sdk::env::random_seed_array() | ||
| pub fn sign(&mut self, payload: [u8; 32], path: String) -> Promise { | ||
| self.pending_requests.insert(&payload, &None); |
There was a problem hiding this comment.
This allows two different accounts to submit the same payload simultaneously, but only one of the sign calls will return successfully, and possibly with the wrong signature.
pending_requests should either be keyed with the payload and the predecessor combined, or sign should reject duplicate, simultaneous payloads.
ChaoticTempest
previously approved these changes
Jan 18, 2024
volovyks
approved these changes
Jan 22, 2024
There was a problem hiding this comment.
LGTM
@ChaoticTempest I've fixed the conflict and prevented the submission of the same payload. Take a look and let's merge it.
Terraform Feature Environment Destroy (dev-401)Terraform Initialization ⚙️
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding a prototype of what the self call approach described in near/NEPs#516 (comment) would look like:
signfunction recursively calls itself until it runs out of gasrespondfunction can add the requested signatures and allowsignto return the signatureTo test this implementation:
cargo build -p mpc-contract --target wasm32-unknown-unknown --releasenearup run localnet --num-shards 1 --binary-path <path-for-nearcore-binary>mpc.node0below)env NEAR_ENV=localnet near deploy mpc.node0 <path-to-contract-wasm> init '{"threshold": 1, "participants":{}}' --keyPath ~/.near-credentials/localnet/mpc.node0.jsonenv NEAR_ENV=localnet near call mpc.node0 sign '{"payload":[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]}' --accountId mpc.node0 --keyPath ~/.near-credentials/localnet/mpc.node0.json --gas 300000000000000Observe that it eventually returns a "exceeded prepaid gas" error after recursing 52 times.
Now to see that
respondworks. We can submit a signature byif there is a
signcall running at the same time, it will terminate and returnsomething.