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

Ppe/uncommitted state #311

Merged
merged 5 commits into from
Jan 23, 2023
Merged

Ppe/uncommitted state #311

merged 5 commits into from
Jan 23, 2023

Conversation

ppedziwiatr
Copy link
Contributor

@ppedziwiatr ppedziwiatr commented Jan 4, 2023

#290 🤯

@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch 3 times, most recently from 61c1d10 to 4b8c874 Compare January 5, 2023 16:48
@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch from bab9299 to a8ba348 Compare January 7, 2023 15:25
@ppedziwiatr
Copy link
Contributor Author

image

wtf

@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch 3 times, most recently from 802a8b2 to 6e49d10 Compare January 16, 2023 14:23
@ppedziwiatr ppedziwiatr marked this pull request as ready for review January 16, 2023 14:39
@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch 5 times, most recently from 3552618 to f889faa Compare January 16, 2023 17:12
@@ -73,6 +73,10 @@ export function handle(state, action) {
const recipient = _input.recipient;
const amount = _input.amount;

if (amount == 0 ) {
throw new ContractError('TransferFromZero');
Copy link
Contributor

@rpiszczatowski rpiszczatowski Jan 17, 2023

Choose a reason for hiding this comment

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

Is amount the sender balance (transfer-FROM-zero suggests that)? Shouldn't we check whether amount <= 0?

Copy link
Contributor Author

@ppedziwiatr ppedziwiatr Jan 17, 2023

Choose a reason for hiding this comment

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

In a real contract - yes.

This contract is a simplified, stripped-down version (I believe there's a warn in the header -

* This is an example token contract that mimics the
) used only for testing certain features (like this specific case, where a transfer from zero was an indication of some issue in the internal writes feature in the whole test scenario)

for (i = 0; i < string.length; i++) {
chr = string.charCodeAt(i);
hash = (hash << 5) - hash + chr;
hash |= 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't |= 0 basically noop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

again, it's a compiled code (probably generated by the TS compiler), so hard for me to say.

BUT...I believe the idea here is to use '0' as the 'default' value, in case the previous assignments would result in sth like NaN, null, or whatever.

@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch 2 times, most recently from 794898e to ce0271d Compare January 17, 2023 09:02
@ppedziwiatr
Copy link
Contributor Author

1.2.48-beta.1

@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch from 2366f9d to 198d013 Compare January 23, 2023 13:52
@ppedziwiatr ppedziwiatr force-pushed the ppe/uncommitted-state branch from 198d013 to 6cd7ce6 Compare January 23, 2023 14:06
@ppedziwiatr ppedziwiatr merged commit f1790fa into main Jan 23, 2023
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.

None yet

2 participants