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

En 7630/esdt new built in funcs #2332

Merged
merged 14 commits into from
Oct 6, 2020

Conversation

sasurobert
Copy link
Contributor

No description provided.

Copy link
Contributor

@SebastianMarian SebastianMarian left a comment

Choose a reason for hiding this comment

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

Need some unit tests for these new builtin functions

return e, nil
}

// ProcessBuiltinFunction resolve ESDT function calls
Copy link
Contributor

Choose a reason for hiding this comment

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

resolves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return e, nil
}

// ProcessBuiltinFunction resolve ESDT function calls
Copy link
Contributor

Choose a reason for hiding this comment

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

resolves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return e, nil
}

// ProcessBuiltinFunction resolve ESDT function calls
Copy link
Contributor

Choose a reason for hiding this comment

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

resolves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

func (e *esdtPause) togglePause(_ state.UserAccountHandler, _ []byte) error {

Copy link
Contributor

Choose a reason for hiding this comment

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

remove empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -43,20 +43,20 @@ func NewESDTTransferFunc(
return e, nil
}

// ProcessBuiltinFunction will transfer the underlying esdt balance of the account
// ProcessBuiltinFunction resolve ESDT function calls
Copy link
Contributor

Choose a reason for hiding this comment

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

resolves

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ccorcoveanu ccorcoveanu self-requested a review September 30, 2020 06:25
}

// IsPayableHandler provides IsPayable function which returns if an account is payable or not
type IsPayableHandler interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

PayableHandler is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

var ErrESDTIsFrozenForAccount = errors.New("account is frozen for this esdt token")

// ErrNilIsPayableHandler signals that nil isPayableHandler was provided
var ErrNilIsPayableHandler = errors.New("nil isPayableHandler was provided")
Copy link
Contributor

Choose a reason for hiding this comment

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

ErrNilPayableHandler is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

package mock

// IsPayableHandlerStub -
type IsPayableHandlerStub struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

PayableHandlerStub is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

// NewESDTTransferFunc returns the esdt transfer built-in function component
func NewESDTTransferFunc(
funcGasCost uint64,
marshalizer marshal.Marshalizer,
pauseHandler process.ESDTPauseHandler,
Copy link
Contributor

Choose a reason for hiding this comment

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

payableHandler is not sent and set but is used below. If there would have been some unit tests this would have not been happened :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

payableHandler is given via SET function. It is a chicken and egg problem in the constructors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -187,6 +217,15 @@ func getESDTDataFromKey(
return esdtData, nil
}

func (e *esdtTransfer) setIsPayable(isPayableHandler process.IsPayableHandler) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

payableHandler process.PayableHandler is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -154,3 +156,20 @@ func createGasConfig(gasMap map[string]map[string]uint64) (*GasCost, error) {

return &gasCost, nil
}

// SetIsPayableHandler sets the payable interface to the needed functions
func SetIsPayableHandler(container process.BuiltInFunctionContainer, isPayableHandler process.IsPayableHandler) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

func SetPayableHandler(container process.BuiltInFunctionContainer, payableHandler process.PayableHandler)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bytes Value = 1 [(gogoproto.jsontag) = "value", (gogoproto.casttypewith) = "math/big.Int;github.com/ElrondNetwork/elrond-go/data.BigIntCaster"];
bool Frozen = 2 [(gogoproto.jsontag) = "frozen"];
bytes Value = 1 [(gogoproto.jsontag) = "value", (gogoproto.casttypewith) = "math/big.Int;github.com/ElrondNetwork/elrond-go/data.BigIntCaster"];
bytes Properties = 3 [(gogoproto.jsontag) = "properties"];
Copy link
Contributor

Choose a reason for hiding this comment

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

You deleted -> bool Frozen = 2 [(gogoproto.jsontag) = "frozen"];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if err != nil {
return nil, err
}

return esdtData, nil
}

func (e *esdtTransfer) setIsPayable(isPayableHandler process.IsPayableHandler) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

setPayable or setPayableHandler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

SebastianMarian
SebastianMarian previously approved these changes Oct 1, 2020
const lengthOfESDTMetadata = 2

const (
METADATA_PAUSED = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to camel case, that's how we use constants everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

const (
METADATA_FROZEN = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move constants up, too have them in the same place? Wold provide a better overview on the possible metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

// NewESDTFreezeWipeFunc returns the esdt freeze/un-freeze/wipe built-in function component
func NewESDTFreezeWipeFunc(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename this, so it won't have both freeze and wipe in the name. But I don't have a good enough suggestion right now, so if you also don't have a better idea, it can stay like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, I did not have any good idea.

@sasurobert sasurobert merged commit 0545bab into feat/ESDT-new-arwen Oct 6, 2020
@sasurobert sasurobert deleted the EN-7630/ESDT-new-builtInFuncs branch October 6, 2020 09:22
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

3 participants