Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

Handle CALL_ID == CALLER_ID not ACCT_ID #252

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iamchrissmith
Copy link
Contributor

In writing the proof for DS-auth, I need to cover the condition where CALLER_ID == ACCT_ID this was creating a problem in displaying the results because call_id was CALLER_ID but the v2n object was still expecting ACCT_ID.

To correct this I abstracted the getting of the contract to a helper function and added a condition to handle if the CALLER_ID == ACCT_ID and CALLER_ID is passed as CALL_ID.

Let me know if this is too narrow of a solution or would cause problems in cases outside my situation.

@livnev livnev requested a review from mhhf July 11, 2019 12:46
@iamchrissmith
Copy link
Contributor Author

i see that the buildbot is failing. The error is HEAD not based on 'origin/master' which I'm guessing is based on the fact that i'm working off my fork. I did that since i don't have permission to push to this repo. Let me know if i need to do something else to get buildbot to be happy

@MrChico
Copy link
Member

MrChico commented Jul 12, 2019

Hey @iamchrissmith the buildbot fail happens since your a little behind master. If you rebase on top of master you should get a nice buildbot test and we can merge this

@iamchrissmith iamchrissmith force-pushed the handle-get-contract-for-caller-id branch from 167f8de to 0ed666b Compare July 12, 2019 16:04
@iamchrissmith
Copy link
Contributor Author

iamchrissmith commented Jul 12, 2019

also, i saw this comment:

klab/lib/evmv.js

Lines 45 to 46 in 0ed666b

// TODO - use var to contractname map to get the contractname
// and use it to get the right contract here

Since i'm touching this piece of code, I'd be happy to do that here, but I wasn't sure what exactly it was referring to. LMK if I can/should add something in for that

@iamchrissmith iamchrissmith force-pushed the handle-get-contract-for-caller-id branch from 0ed666b to 4c43528 Compare July 12, 2019 23:33
@MrChico
Copy link
Member

MrChico commented Jul 13, 2019

There's other cases where this occurs as well. Whenever ACCT_ID has been stipulated to be equal to smth else, the <id> cell can take hold a different value, so the contract lookup fails. I just ran into this with <id> Owner </id>.

Might be a better idea to look at what the value of the <id> cell is at the first node and set that to be the contract name, for a more general solution

@iamchrissmith iamchrissmith force-pushed the handle-get-contract-for-caller-id branch from 4c43528 to cf73ca0 Compare July 15, 2019 14:56
@iamchrissmith
Copy link
Contributor Author

iamchrissmith commented Jul 15, 2019

Is that not what it already happening here: https://github.com/dapphub/klab/blob/master/lib/clean-evm-node.js#L5?

I'm not seeing a situation in my FV out dir where <id> is set to anything other than ACCT_ID so I may be looking in the wrong place or not understanding the structure of the state when its being parsed

move getting the contract to a helper function and add a condition to handle if the CALLER_ID == ACCT_ID and CALLER_ID is passed as CALL_ID
@iamchrissmith iamchrissmith force-pushed the handle-get-contract-for-caller-id branch from cf73ca0 to 836b26f Compare July 16, 2019 15:13
@iamchrissmith
Copy link
Contributor Author

@MrChico Should we just set it to ACCT_ID or will the v2n have other call_ids that we need to track? I just ran into one where it was set to Owner but should be looking up with ACCT_ID

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants