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.

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

Equation: 120, when w=JUMPI, outcome of JUMPI depends on µs[1] too, so the exception Z and its coding #151

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

efaysal
Copy link

@efaysal efaysal commented Jan 9, 2016

In Exceptional Halting: Equation: 120,

(w ∈ {JUMP, JUMPI} ∧ µs[0] not in D(Ib)) --^^--

should be separated as the outcome of JUMPI is also depending on the value of µs[1]. Better to use the following:

(w =JUMP ∧ µs[0] not in D(Ib)) V (w =JUMPI ∧ µs[1] != 0 ∧ µs[0] not in D(Ib)) ... if µs[1] = 0, JUMPI is just shifting the pc by one, and it should be an opcode not data assuming that pc+1 is less than the total number of opdodes in Ib.

--^^-- comes to:

(w =JUMP ∧ µs[0] not in D(Ib)) V (w =JUMPI ∧ µs[1] != 0 ∧ µs[0] not in D(Ib)) V (w =JUMPI ∧ µs[1] = 0 ∧ pc+1 > Ic|), Paper.tex has my committed changes.

Also is the C++ coding of JUMPI used µs[1]?

Implicitly the condition related to µs[1] is tested in "if (m_stack[m_stack.size() - 2])", is this robust?

At least the topmost two elements from the stack should exist if not an issue is there because JUMPI needs, accordingly to the value of µs[1], to decide to jump to µs[0] or to just increment by one the current pc.

Much safe to test the existence of µs[1] && if (m_stack.size()>2) to decide.

case Instruction::JUMPI:
if (m_stack[m_stack.size() - 2]) \ only OK when m_stack.size() >2 ...
nextPC = verifyJumpDest(m_stack.back(), m_jumpDests);
m_stack.pop_back();
m_stack.pop_back();
break;

@gavofyork
Copy link
Contributor

pc+1 > |c| is unnecessary; that case is already guaranteed valid by virtue of the explicit assertion that any value of code outside the specific code portion given (c) is defined as 0/STOP. Otherwise, this is sensible and should go in. Please reformulate accordingly and I'll merge.

@efaysal
Copy link
Author

efaysal commented Feb 22, 2016

Done!

@gavofyork
Copy link
Contributor

needs rebasing/merging

@efaysal
Copy link
Author

efaysal commented Mar 3, 2016

I guess it's OK now:
git checkout master
Already on 'master'
Your branch is up-to-date with 'origin/master'.

@gavofyork
Copy link
Contributor

still needs rebasing/merging

@axic
Copy link
Member

axic commented Mar 23, 2017

@efaysal can you please rebase?

@efaysal
Copy link
Author

efaysal commented Mar 23, 2017

After a number of trails, It shows this "This branch has no conflicts with the base branch
Only those with write access to this repository can merge pull requests." .

@efaysal
Copy link
Author

efaysal commented Mar 23, 2017

I tried to fix the issues with Travis, I got this,

remote: Permission to ethereum/yellowpaper.git denied to efaysal.

@gavofyork
Copy link
Contributor

this is now broken. please ensure it is merged properly. if you're unsure how to do this, please read up on how git works.

@efaysal
Copy link
Author

efaysal commented Mar 24, 2017

I only need to reread a Pull Request Tutorial, indeed.

Copy link
Contributor

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

The identified issue needs correcting, but this PR still needs a proper resolution/rebase.

@efaysal
Copy link
Author

efaysal commented Mar 30, 2017

I'm sharing with you the steps, everything was OK. However when I pushed as it was required,
I got this The requested URL returned error: 403. The logs of my steps are below,

faysal@faysal-VPCEH12FX:$ cd yellowpaper
faysal@faysal-VPCEH12FX:
/yellowpaper$ git remote add upstream1 https://github.com/ethereum/yellowpaper
faysal@faysal-VPCEH12FX:/yellowpaper$ git remote -v
origin https://github.com/ethereum/yellowpaper (fetch)
origin https://github.com/ethereum/yellowpaper (push)
upstream https://github.com/ethereum/yellowpaper (fetch)
upstream https://github.com/ethereum/yellowpaper (push)
upstream1 https://github.com/ethereum/yellowpaper (fetch)
upstream1 https://github.com/ethereum/yellowpaper (push)
faysal@faysal-VPCEH12FX:
/yellowpaper$ git fetch upstream1
From https://github.com/ethereum/yellowpaper

  • [new branch] deploytest -> upstream1/deploytest
  • [new branch] frontier -> upstream1/frontier
  • [new branch] gh-pages -> upstream1/gh-pages
  • [new branch] invalid_is_invalid -> upstream1/invalid_is_invalid
  • [new branch] master -> upstream1/master
  • [new branch] metropolis -> upstream1/metropolis
  • [new branch] poc-4 -> upstream1/poc-4
  • [new branch] superfluous -> upstream1/superfluous
    faysal@faysal-VPCEH12FX:/yellowpaper$ git reset --hard upstream1/superfluous
    HEAD is now at a3381de Various typos fixed thanks to 看K线图.
    faysal@faysal-VPCEH12FX:
    /yellowpaper$ git checkout master
    Previous HEAD position was a3381de... Various typos fixed thanks to 看K线图.
    Switched to branch 'master'
    Your branch is ahead of 'origin/master' by 1 commit.
    (use "git push" to publish your local commits)
    faysal@faysal-VPCEH12FX:/yellowpaper$ git add --all
    faysal@faysal-VPCEH12FX:
    /yellowpaper$ git commit -m "Initial commit-"
    On branch master
    Your branch is ahead of 'origin/master' by 1 commit.
    (use "git push" to publish your local commits)

You are currently rebasing.

nothing to commit, working directory clean
faysal@faysal-VPCEH12FX:~/yellowpaper$ git push -u origin master
Username for 'xxxxx'
Password for 'xxxxx':
remote: Permission to ethereum/yellowpaper.git denied to efaysal.
fatal: unable to access 'https://github.com/ethereum/yellowpaper/': The requested URL returned error: 403

@fulldecent
Copy link
Contributor

This PR is getting all messed up.

Recommendation:

  • Make a backup of your working directory
  • Delete your fork on github
  • Refork on github
  • Redo the changes
  • git push --force

@@ -451,7 +451,11 @@ \subsubsection{Block Header Validity}
\begin{equation}
D(H) \equiv \begin{dcases}
\mindifficulty & \text{if} \quad H_i = 0\\
<<<<<<< HEAD
\text{max}\!\left(\mindifficulty, {P(H)_H}_d + \diffadjustment\times\frontiermod + \expdiffsymb \right) & \text{if} \quad H_i<\mathit{TBD}\\
Copy link
Contributor

@jamesray1 jamesray1 Feb 17, 2018

Choose a reason for hiding this comment

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

I don't think this is changing anything compared to the latest (indeed it's outdated): \text{max}\!\left(\mindifficulty, {P(H)_{H}}_{\mathrm{d}} + \diffadjustment\times\homesteadmod + \expdiffsymb \right) & \text{otherwise}\\

@@ -769,11 +773,15 @@ \section{Message Call} \label{ch:call}
\boldsymbol{\sigma} & \text{if} \quad \boldsymbol{\sigma}^{**} = \varnothing \\
\boldsymbol{\sigma}^{**} & \text{otherwise}
\end{cases} \\
<<<<<<< HEAD
(\boldsymbol{\sigma}^{**}, g', \mathbf{s}, \mathbf{o}) & \equiv & \begin{cases}
Copy link
Contributor

@jamesray1 jamesray1 Feb 17, 2018

Choose a reason for hiding this comment

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

Currently we have:

yellowpaper/Paper.tex

Lines 817 to 829 in fb260c1

\begin{eqnarray}
\boldsymbol{\sigma}' & \equiv & \begin{cases}
\boldsymbol{\sigma} & \text{if} \quad \boldsymbol{\sigma}^{**} = \varnothing \\
\boldsymbol{\sigma}^{**} & \text{otherwise}
\end{cases} \\
g' & \equiv & \begin{cases}
0 & \text{if} \quad \boldsymbol{\sigma}^{**} = \varnothing \ \wedge \ \mathbf{o} = \varnothing \\
g^{**} & \text{otherwise}
\end{cases} \\ \nonumber
z & \equiv & \begin{cases}
0 & \text{if} \quad \boldsymbol{\sigma}^{**} = \varnothing \\
1 & \text{otherwise}
\end{cases} \\

I'm not sure what the benefit of including this is.

\mathbf{\delta}_w = \varnothing \quad \vee \nonumber \\
\lVert\boldsymbol{\mu}_\mathbf{s}\rVert < \mathbf{\delta}_w \quad \vee \nonumber \\
( w = \text{\small JUMP} \quad \wedge \quad \boldsymbol{\mu}_\mathbf{s}[0] \notin D(I_\mathbf{b}) ) \quad \vee \nonumber \\
( w = \text{\small JUMPI} \quad \wedge \quad \boldsymbol{\mu}_\mathbf{s}[1] \neq 0 \quad \wedge \quad \boldsymbol{\mu}_\mathbf{s}[0] \notin D(I_\mathbf{b}) ) \quad \vee \nonumber \\
Copy link
Contributor

@jamesray1 jamesray1 Feb 17, 2018

Choose a reason for hiding this comment

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

@efaysal, I put 919-920 into #616

jamesray1 added a commit to jamesray1/yellowpaper that referenced this pull request Feb 22, 2018
pirapira added a commit that referenced this pull request Feb 22, 2018
Pull in the changes from #616 that are from #151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants