Skip to content

Fix Uint256 Uint160#1407

Closed
Tommo-L wants to merge 2 commits intoneo-project:masterfrom
Tommo-L:fix_uint256_uint160
Closed

Fix Uint256 Uint160#1407
Tommo-L wants to merge 2 commits intoneo-project:masterfrom
Tommo-L:fix_uint256_uint160

Conversation

@Tommo-L
Copy link
Copy Markdown
Contributor

@Tommo-L Tommo-L commented Jan 10, 2020

relate to #1387

vncoelho
vncoelho previously approved these changes Jan 10, 2020
@vncoelho vncoelho dismissed their stale review January 10, 2020 11:42

Re-thinking if we should merge this PR on @john-h-k

@vncoelho
Copy link
Copy Markdown
Member

@Tommo-L, perhaps we could merge this PR on #1387, since @john-h-k and @MithrilMan started the topic.

Span<byte> dst = new Span<byte>(p, Length);
value[..Length].CopyTo(dst);
}
Span<byte> dst = MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value1, Length / sizeof(ulong)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use the same schema as Uint160?

@Tommo-L
Copy link
Copy Markdown
Contributor Author

Tommo-L commented Jan 10, 2020

@vncoelho Need your help, I couldn't send commit directly. And #1387 needs some fixes, which didn't pass the ut. Could you add to it, and I'll close this PR.

@Tommo-L
Copy link
Copy Markdown
Contributor Author

Tommo-L commented Jan 10, 2020

Why not use the same schema as Uint160?

@shargon Uint160 has two different types(ulong and uint)

@vncoelho
Copy link
Copy Markdown
Member

Merged @Tommo-L, I cloned his repo and added yours as upstream.

Then, I checked into his branch git checkout patch-1 and merged with git merge upstream fix_uint256_uint160

@Tommo-L
Copy link
Copy Markdown
Contributor Author

Tommo-L commented Jan 10, 2020

Thank you 😊

@Tommo-L Tommo-L closed this Jan 10, 2020
@Tommo-L Tommo-L deleted the fix_uint256_uint160 branch January 10, 2020 13:54
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.

3 participants